On Mon, Oct 28, 2019 at 09:03:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Actually call namecheck on attribute names before we hand them over to
> userspace.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> fs/xfs/libxfs/xfs_attr_leaf.h | 4 +--
> fs/xfs/xfs_attr_list.c | 60 +++++++++++++++++++++++++++--------------
> 2 files changed, 41 insertions(+), 23 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
> index 7b74e18becff..bb0880057ee3 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.h
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.h
> @@ -67,8 +67,8 @@ int xfs_attr3_leaf_add(struct xfs_buf *leaf_buffer,
> struct xfs_da_args *args);
> int xfs_attr3_leaf_remove(struct xfs_buf *leaf_buffer,
> struct xfs_da_args *args);
> -void xfs_attr3_leaf_list_int(struct xfs_buf *bp,
> - struct xfs_attr_list_context *context);
> +int xfs_attr3_leaf_list_int(struct xfs_buf *bp,
> + struct xfs_attr_list_context *context);
>
> /*
> * Routines used for shrinking the Btree.
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 00758fdc2fec..c02f22d50e45 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -49,14 +49,16 @@ xfs_attr_shortform_compare(const void *a, const void *b)
> * we can begin returning them to the user.
> */
> static int
> -xfs_attr_shortform_list(xfs_attr_list_context_t *context)
> +xfs_attr_shortform_list(
> + struct xfs_attr_list_context *context)
> {
> - attrlist_cursor_kern_t *cursor;
> - xfs_attr_sf_sort_t *sbuf, *sbp;
> - xfs_attr_shortform_t *sf;
> - xfs_attr_sf_entry_t *sfe;
> - xfs_inode_t *dp;
> - int sbsize, nsbuf, count, i;
> + struct attrlist_cursor_kern *cursor;
> + struct xfs_attr_sf_sort *sbuf, *sbp;
> + struct xfs_attr_shortform *sf;
> + struct xfs_attr_sf_entry *sfe;
> + struct xfs_inode *dp;
> + int sbsize, nsbuf, count, i;
> + int error = 0;
>
> ASSERT(context != NULL);
> dp = context->dp;
> @@ -84,6 +86,11 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
> (XFS_ISRESET_CURSOR(cursor) &&
> (dp->i_afp->if_bytes + sf->hdr.count * 16) < context->bufsize)) {
> for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
> + if (!xfs_attr_namecheck(sfe->nameval, sfe->namelen)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> + context->dp->i_mount);
> + return -EFSCORRUPTED;
> + }
> context->put_listent(context,
> sfe->flags,
> sfe->nameval,
> @@ -161,10 +168,8 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
> break;
> }
> }
> - if (i == nsbuf) {
> - kmem_free(sbuf);
> - return 0;
> - }
> + if (i == nsbuf)
> + goto out;
>
> /*
> * Loop putting entries into the user buffer.
> @@ -174,6 +179,12 @@ 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);
> + error = -EFSCORRUPTED;
> + goto out;
> + }
> context->put_listent(context,
> sbp->flags,
> sbp->name,
> @@ -183,9 +194,9 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
> break;
> cursor->offset++;
> }
> -
> +out:
> kmem_free(sbuf);
> - return 0;
> + return error;
> }
>
> /*
> @@ -284,7 +295,7 @@ xfs_attr_node_list(
> struct xfs_buf *bp;
> struct xfs_inode *dp = context->dp;
> struct xfs_mount *mp = dp->i_mount;
> - int error;
> + int error = 0;
>
> trace_xfs_attr_node_list(context);
>
> @@ -358,7 +369,9 @@ xfs_attr_node_list(
> */
> for (;;) {
> leaf = bp->b_addr;
> - xfs_attr3_leaf_list_int(bp, context);
> + error = xfs_attr3_leaf_list_int(bp, context);
> + if (error)
> + break;
> xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
> if (context->seen_enough || leafhdr.forw == 0)
> break;
> @@ -369,13 +382,13 @@ xfs_attr_node_list(
> return error;
> }
> xfs_trans_brelse(context->tp, bp);
> - return 0;
> + return error;
> }
>
> /*
> * Copy out attribute list entries for attr_list(), for leaf attribute lists.
> */
> -void
> +int
> xfs_attr3_leaf_list_int(
> struct xfs_buf *bp,
> struct xfs_attr_list_context *context)
> @@ -417,7 +430,7 @@ xfs_attr3_leaf_list_int(
> }
> if (i == ichdr.count) {
> trace_xfs_attr_list_notfound(context);
> - return;
> + return 0;
> }
> } else {
> entry = &entries[0];
> @@ -457,6 +470,11 @@ xfs_attr3_leaf_list_int(
> valuelen = be32_to_cpu(name_rmt->valuelen);
> }
>
> + if (!xfs_attr_namecheck(name, namelen)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> + context->dp->i_mount);
> + return -EFSCORRUPTED;
> + }
> context->put_listent(context, entry->flags,
> name, namelen, valuelen);
> if (context->seen_enough)
> @@ -464,7 +482,7 @@ xfs_attr3_leaf_list_int(
> cursor->offset++;
> }
> trace_xfs_attr_list_leaf_end(context);
> - return;
> + return 0;
> }
>
> /*
> @@ -483,9 +501,9 @@ xfs_attr_leaf_list(xfs_attr_list_context_t *context)
> if (error)
> return error;
>
> - xfs_attr3_leaf_list_int(bp, context);
> + error = xfs_attr3_leaf_list_int(bp, context);
> xfs_trans_brelse(context->tp, bp);
> - return 0;
> + return error;
> }
>
> int
>
