public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Bill Wendling <morbo@google.com>
Cc: linux-kernel@vger.kernel.org, Carlos Maiolino <cem@kernel.org>,
	Gogul Balakrishnan <bgogul@google.com>,
	Arman Hasanzadeh <armanihm@google.com>,
	Kees Cook <kees@kernel.org>,
	linux-xfs@vger.kernel.org, codemender-patching+linux@google.com
Subject: Re: [PATCH] xfs: annotate struct xfs_attr_list_context with __counted_by_ptr
Date: Mon, 2 Mar 2026 21:14:19 -0800	[thread overview]
Message-ID: <20260303051419.GD57948@frogsfrogsfrogs> (raw)
In-Reply-To: <20260303015646.2796170-1-morbo@google.com>

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@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
> ---
>  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?

--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
> 
> 

  reply	other threads:[~2026-03-03  5:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-03  1:56 [PATCH] xfs: annotate struct xfs_attr_list_context with __counted_by_ptr Bill Wendling
2026-03-03  5:14 ` Darrick J. Wong [this message]
2026-03-03  7:35   ` Bill Wendling
2026-03-03 14:40 ` Christoph Hellwig
2026-03-16 18:42   ` Bill Wendling
2026-03-16 18:41 ` [PATCH v2] " Bill Wendling
2026-03-16 22:53   ` Darrick J. Wong
2026-03-17  9:13   ` Christoph Hellwig
2026-03-18 10:12   ` Carlos Maiolino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260303051419.GD57948@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=armanihm@google.com \
    --cc=bgogul@google.com \
    --cc=cem@kernel.org \
    --cc=codemender-patching+linux@google.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=morbo@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox