Re: [PATCH 05/33] xfs: remove the ATTR_INCOMPLETE flag

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Dec 12, 2019 at 11:54:05AM +0100, Christoph Hellwig wrote:
> Replace the ATTR_INCOMPLETE flag with a new boolean field in struct
> xfs_attr_list_context.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_attr.h | 5 ++---
>  fs/xfs/scrub/attr.c      | 2 +-
>  fs/xfs/xfs_attr_list.c   | 6 +-----
>  3 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 91c2cb14276e..04a628016728 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -36,11 +36,10 @@ struct xfs_attr_list_context;
>  #define ATTR_KERNOTIME	0x1000	/* [kernel] don't update inode timestamps */
>  #define ATTR_KERNOVAL	0x2000	/* [kernel] get attr size only, not value */
>  
> -#define ATTR_INCOMPLETE	0x4000	/* [kernel] return INCOMPLETE attr keys */

Hmm, did we used to allow ATTR_INCOMPLETE from the attr_multi userspace
calls?  Maybe we should leave ATTR_INCOMPLETE so that we can blacklist
it in case we ever see it coming from userspace?  Or at least prevent
anyone from reusing 0x4000 and suffering the confusion.

I also wonder if we can break userspace this way, but OTOH userspace
should never be able to query incomplete attrs and this is an obscure
ioctl anyway so maybe it's fine....

--D

>  #define ATTR_ALLOC	0x8000	/* [kernel] allocate xattr buffer on demand */
>  
>  #define ATTR_KERNEL_FLAGS \
> -	(ATTR_KERNOTIME | ATTR_KERNOVAL | ATTR_INCOMPLETE | ATTR_ALLOC)
> +	(ATTR_KERNOTIME | ATTR_KERNOVAL | ATTR_ALLOC)
>  
>  #define XFS_ATTR_FLAGS \
>  	{ ATTR_DONTFOLLOW, 	"DONTFOLLOW" }, \
> @@ -51,7 +50,6 @@ struct xfs_attr_list_context;
>  	{ ATTR_REPLACE,		"REPLACE" }, \
>  	{ ATTR_KERNOTIME,	"KERNOTIME" }, \
>  	{ ATTR_KERNOVAL,	"KERNOVAL" }, \
> -	{ ATTR_INCOMPLETE,	"INCOMPLETE" }, \
>  	{ ATTR_ALLOC,		"ALLOC" }
>  
>  /*
> @@ -123,6 +121,7 @@ typedef struct xfs_attr_list_context {
>  	 * error values to the xfs_attr_list caller.
>  	 */
>  	int				seen_enough;
> +	bool				allow_incomplete;
>  
>  	ssize_t				count;		/* num used entries */
>  	int				dupcnt;		/* count dup hashvals seen */
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index d9f0dd444b80..d804558cdbca 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -497,7 +497,7 @@ xchk_xattr(
>  	sx.context.resynch = 1;
>  	sx.context.put_listent = xchk_xattr_listent;
>  	sx.context.tp = sc->tp;
> -	sx.context.flags = ATTR_INCOMPLETE;
> +	sx.context.allow_incomplete = true;
>  	sx.sc = sc;
>  
>  	/*
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index d37743bdf274..5139ef983cd6 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -452,7 +452,7 @@ xfs_attr3_leaf_list_int(
>  		}
>  
>  		if ((entry->flags & XFS_ATTR_INCOMPLETE) &&
> -		    !(context->flags & ATTR_INCOMPLETE))
> +		    !context->allow_incomplete)
>  			continue;		/* skip incomplete entries */
>  
>  		if (entry->flags & XFS_ATTR_LOCAL) {
> @@ -632,10 +632,6 @@ xfs_attr_list(
>  	    (cursor->hashval || cursor->blkno || cursor->offset))
>  		return -EINVAL;
>  
> -	/* Only internal consumers can retrieve incomplete attrs. */
> -	if (flags & ATTR_INCOMPLETE)
> -		return -EINVAL;
> -
>  	/*
>  	 * Check for a properly aligned buffer.
>  	 */
> -- 
> 2.20.1
>