All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.