[v2] xfs: annotate struct xfs_attr_list_context with __counted_by_ptr

[PATCH v2] xfs: annotate struct xfs_attr_list_context with __counted_by_ptr

Posted by Bill Wendling 3 weeks ago

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@google.com>
---
Cc: Carlos Maiolino <cem@kernel.org>
Cc: "Darrick J. Wong" <djwong@kernel.org>
Cc: Gogul Balakrishnan <bgogul@google.com>
Cc: Arman Hasanzadeh <armanihm@google.com>
Cc: Kees Cook <kees@kernel.org>
Cc: linux-xfs@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: codemender-patching+linux@google.com
---
v2 - Place comment in a more readable spot.
---
 fs/xfs/libxfs/xfs_attr.h | 3 ++-
 fs/xfs/xfs_handle.c      | 2 +-
 fs/xfs/xfs_xattr.c       | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 8244305949de..4b4217e23d1c 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -55,7 +55,8 @@ 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 */
+	/* output buffer */
+	void			*buffer __counted_by_ptr(bufsize);
 
 	/*
 	 * 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.851.ga537e3e6e9-goog

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

Posted by Carlos Maiolino 2 weeks, 5 days ago

On Mon, 16 Mar 2026 18:41:58 +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)
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: annotate struct xfs_attr_list_context with __counted_by_ptr
      commit: e5966096d0856d071269cb5928d6bc33342d2dfd

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

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

Posted by Christoph Hellwig 2 weeks, 6 days ago

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Posted by Darrick J. Wong 3 weeks ago

On Mon, Mar 16, 2026 at 06:41:58PM +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@google.com>

Looks ok to me, even if it's nothing earthshattering ;)

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> Cc: Carlos Maiolino <cem@kernel.org>
> Cc: "Darrick J. Wong" <djwong@kernel.org>
> Cc: Gogul Balakrishnan <bgogul@google.com>
> Cc: Arman Hasanzadeh <armanihm@google.com>
> Cc: Kees Cook <kees@kernel.org>
> Cc: linux-xfs@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: codemender-patching+linux@google.com
> ---
> v2 - Place comment in a more readable spot.
> ---
>  fs/xfs/libxfs/xfs_attr.h | 3 ++-
>  fs/xfs/xfs_handle.c      | 2 +-
>  fs/xfs/xfs_xattr.c       | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 8244305949de..4b4217e23d1c 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -55,7 +55,8 @@ 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 */
> +	/* output buffer */
> +	void			*buffer __counted_by_ptr(bufsize);
>  
>  	/*
>  	 * 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.851.ga537e3e6e9-goog
> 
>