Re: [PATCH 2/4] xfs: namecheck attribute names before listing them

From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/4] xfs: namecheck attribute names before listing them
Date: Mon, 28 Oct 2019 14:18:57 -0400	[thread overview]
Message-ID: <20191028181857.GC26529@bfoster> (raw)
In-Reply-To: <157198049955.2873445.974102983711142585.stgit@magnolia>

On Thu, Oct 24, 2019 at 10:14:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Actually call namecheck on attribute names before we hand them over to
> userspace.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.h |    4 ++--
>  fs/xfs/xfs_attr_list.c        |   40 ++++++++++++++++++++++++++++++++--------
>  2 files changed, 34 insertions(+), 10 deletions(-)
> 
> 
...
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 00758fdc2fec..3a1158a1347d 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
...
> @@ -174,6 +179,11 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  			cursor->hashval = sbp->hash;
>  			cursor->offset = 0;
>  		}
> +		if (!xfs_attr_namecheck(sbp->name, sbp->namelen)) {
> +			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +					 context->dp->i_mount);
> +			return -EFSCORRUPTED;
> +		}

It looks like we still need to handle freeing sbuf in this path.

>  		context->put_listent(context,
>  				     sbp->flags,
>  				     sbp->name,
...
> @@ -557,6 +574,13 @@ xfs_attr_put_listent(
>  	ASSERT(context->firstu >= sizeof(*alist));
>  	ASSERT(context->firstu <= context->bufsize);
>  
> +	if (!xfs_attr_namecheck(name, namelen)) {
> +		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +				 context->dp->i_mount);
> +		context->seen_enough = -EFSCORRUPTED;
> +		return;
> +	}
> +

Any reason we call this here and the ->put_listent() callers?

Brian

>  	/*
>  	 * Only list entries in the right namespace.
>  	 */
> 