Re: [PATCH] xfs: annotate struct xfs_attr_list_context with __counted_by_ptr

[Bill Wendling]

On Mon, Mar 2, 2026 at 9:14 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Tue, Mar 03, 2026 at 01:56:35AM +0000, Bill Wendling wrote:
> > Add the `__counted_by_ptr` attribute to the `buffer` field of `struct
> > xfs_attr_list_context`. This field is used to point to a buffer of
> > size `bufsize`.
> >
> > The `buffer` field is assigned in:
> > 1. `xfs_ioc_attr_list` in `fs/xfs/xfs_handle.c`
> > 2. `xfs_xattr_list` in `fs/xfs/xfs_xattr.c`
> > 3. `xfs_getparents` in `fs/xfs/xfs_handle.c` (implicitly initialized to NULL)
> >
> > In `xfs_ioc_attr_list`, `buffer` was assigned before `bufsize`. Reorder
> > them to ensure `bufsize` is set before `buffer` is assigned, although
> > no access happens between them.
> >
> > In `xfs_xattr_list`, `buffer` was assigned before `bufsize`. Reorder
> > them to ensure `bufsize` is set before `buffer` is assigned.
> >
> > In `xfs_getparents`, `buffer` is NULL (from zero initialization) and
> > remains NULL. `bufsize` is set to a non-zero value, but since `buffer`
> > is NULL, no access occurs.
> >
> > In all cases, the pointer `buffer` is not accessed before `bufsize` is
> > set.
> >
> > This patch was generated by CodeMender and reviewed by Bill Wendling.
> > Tested by running xfstests.
> >
> > Signed-off-by: Bill Wendling <morbo@xxxxxxxxxx>
> > ---
> > Cc: Carlos Maiolino <cem@xxxxxxxxxx>
> > Cc: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> > Cc: Gogul Balakrishnan <bgogul@xxxxxxxxxx>
> > Cc: Arman Hasanzadeh <armanihm@xxxxxxxxxx>
> > Cc: Kees Cook <kees@xxxxxxxxxx>
> > Cc: linux-xfs@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Cc: codemender-patching+linux@xxxxxxxxxx
> > ---
> >  fs/xfs/libxfs/xfs_attr.h | 2 +-
> >  fs/xfs/xfs_handle.c      | 2 +-
> >  fs/xfs/xfs_xattr.c       | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> > index 8244305949de..4cd161905288 100644
> > --- a/fs/xfs/libxfs/xfs_attr.h
> > +++ b/fs/xfs/libxfs/xfs_attr.h
> > @@ -55,7 +55,7 @@ struct xfs_attr_list_context {
> >       struct xfs_trans        *tp;
> >       struct xfs_inode        *dp;            /* inode */
> >       struct xfs_attrlist_cursor_kern cursor; /* position in list */
> > -     void                    *buffer;        /* output buffer */
> > +     void                    *buffer __counted_by_ptr(bufsize);      /* output buffer */
>
> Looks reasonable, but ... how hard will it be to port __counted_by_ptr
> to userspace?  Files in fs/xfs/libxfs/ get ported to userspace xfs.  I
> see that it maps to an __attribute__.  Does that get us any new gcc
> typechecking magic?
>
I'm not familiar with how the files are ported to user space. There
are #defines in include/uapi/linux/stddef.h that turn this attribute
(and other similarly named attributes) off. Please let me know if
that's not sufficient, as it will most likely apply to other APIs.

As for new typechecking magic, Clang and GCC check to ensure that the
"counter" has an integral type. Otherwise, nothing earth shattering.
:-)

-bw

> --D
>
> >
> >       /*
> >        * Abort attribute list iteration if non-zero.  Can be used to pass
> > diff --git a/fs/xfs/xfs_handle.c b/fs/xfs/xfs_handle.c
> > index d1291ca15239..2b8617ae7ec2 100644
> > --- a/fs/xfs/xfs_handle.c
> > +++ b/fs/xfs/xfs_handle.c
> > @@ -443,8 +443,8 @@ xfs_ioc_attr_list(
> >       context.dp = dp;
> >       context.resynch = 1;
> >       context.attr_filter = xfs_attr_filter(flags);
> > -     context.buffer = buffer;
> >       context.bufsize = round_down(bufsize, sizeof(uint32_t));
> > +     context.buffer = buffer;
> >       context.firstu = context.bufsize;
> >       context.put_listent = xfs_ioc_attr_put_listent;
> >
> > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
> > index a735f16d9cd8..544213067d59 100644
> > --- a/fs/xfs/xfs_xattr.c
> > +++ b/fs/xfs/xfs_xattr.c
> > @@ -332,8 +332,8 @@ xfs_vn_listxattr(
> >       memset(&context, 0, sizeof(context));
> >       context.dp = XFS_I(inode);
> >       context.resynch = 1;
> > -     context.buffer = size ? data : NULL;
> >       context.bufsize = size;
> > +     context.buffer = size ? data : NULL;
> >       context.firstu = context.bufsize;
> >       context.put_listent = xfs_xattr_put_listent;
> >
> > --
> > 2.53.0.473.g4a7958ca14-goog
> >
> >