From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org, hch@lst.de
Subject: Re: [PATCH 2/4] xfs: namecheck attribute names before listing them
Date: Tue, 29 Oct 2019 06:03:57 -0400 [thread overview]
Message-ID: <20191029100357.GC41131@bfoster> (raw)
In-Reply-To: <157232183873.593721.440778415935090240.stgit@magnolia>
On Mon, Oct 28, 2019 at 09:03:58PM -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>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> 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
>
next prev parent reply other threads:[~2019-10-29 10:04 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-29 4:03 [PATCH v2 0/4] xfs: more metadata verifier tightening Darrick J. Wong
2019-10-29 4:03 ` [PATCH 1/4] xfs: check attribute leaf block structure Darrick J. Wong
2019-10-29 10:03 ` Brian Foster
2019-10-29 4:03 ` [PATCH 2/4] xfs: namecheck attribute names before listing them Darrick J. Wong
2019-10-29 10:03 ` Brian Foster [this message]
2019-10-29 4:04 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
2019-10-29 10:04 ` Brian Foster
2019-10-29 4:04 ` [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2019-10-25 5:14 [PATCH 0/4] xfs: more metadata verifier tightening Darrick J. Wong
2019-10-25 5:14 ` [PATCH 2/4] xfs: namecheck attribute names before listing them Darrick J. Wong
2019-10-28 18:18 ` Brian Foster
2019-10-28 18:22 ` Darrick J. Wong
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=20191029100357.GC41131@bfoster \
--to=bfoster@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
/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