* [PATCH 1/4] xfs: check attribute leaf block structure
2019-10-29 4:03 [PATCH v2 0/4] xfs: more metadata verifier tightening Darrick J. Wong
@ 2019-10-29 4:03 ` 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
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2019-10-29 4:03 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, bfoster, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Add missing structure checks in the attribute leaf verifier.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 67 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index f0089e862216..bb80a01bcca9 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -232,6 +232,61 @@ xfs_attr3_leaf_hdr_to_disk(
}
}
+static xfs_failaddr_t
+xfs_attr3_leaf_verify_entry(
+ struct xfs_mount *mp,
+ char *buf_end,
+ struct xfs_attr_leafblock *leaf,
+ struct xfs_attr3_icleaf_hdr *leafhdr,
+ struct xfs_attr_leaf_entry *ent,
+ int idx,
+ __u32 *last_hashval)
+{
+ struct xfs_attr_leaf_name_local *lentry;
+ struct xfs_attr_leaf_name_remote *rentry;
+ char *name_end;
+ unsigned int nameidx;
+ unsigned int namesize;
+ __u32 hashval;
+
+ /* hash order check */
+ hashval = be32_to_cpu(ent->hashval);
+ if (hashval < *last_hashval)
+ return __this_address;
+ *last_hashval = hashval;
+
+ nameidx = be16_to_cpu(ent->nameidx);
+ if (nameidx < leafhdr->firstused || nameidx >= mp->m_attr_geo->blksize)
+ return __this_address;
+
+ /*
+ * Check the name information. The namelen fields are u8 so we can't
+ * possibly exceed the maximum name length of 255 bytes.
+ */
+ if (ent->flags & XFS_ATTR_LOCAL) {
+ lentry = xfs_attr3_leaf_name_local(leaf, idx);
+ namesize = xfs_attr_leaf_entsize_local(lentry->namelen,
+ be16_to_cpu(lentry->valuelen));
+ name_end = (char *)lentry + namesize;
+ if (lentry->namelen == 0)
+ return __this_address;
+ } else {
+ rentry = xfs_attr3_leaf_name_remote(leaf, idx);
+ namesize = xfs_attr_leaf_entsize_remote(rentry->namelen);
+ name_end = (char *)rentry + namesize;
+ if (rentry->namelen == 0)
+ return __this_address;
+ if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
+ rentry->valueblk == 0)
+ return __this_address;
+ }
+
+ if (name_end > buf_end)
+ return __this_address;
+
+ return NULL;
+}
+
static xfs_failaddr_t
xfs_attr3_leaf_verify(
struct xfs_buf *bp)
@@ -240,7 +295,10 @@ xfs_attr3_leaf_verify(
struct xfs_mount *mp = bp->b_mount;
struct xfs_attr_leafblock *leaf = bp->b_addr;
struct xfs_attr_leaf_entry *entries;
+ struct xfs_attr_leaf_entry *ent;
+ char *buf_end;
uint32_t end; /* must be 32bit - see below */
+ __u32 last_hashval = 0;
int i;
xfs_failaddr_t fa;
@@ -273,8 +331,13 @@ xfs_attr3_leaf_verify(
(char *)bp->b_addr + ichdr.firstused)
return __this_address;
- /* XXX: need to range check rest of attr header values */
- /* XXX: hash order check? */
+ buf_end = (char *)bp->b_addr + mp->m_attr_geo->blksize;
+ for (i = 0, ent = entries; i < ichdr.count; ent++, i++) {
+ fa = xfs_attr3_leaf_verify_entry(mp, buf_end, leaf, &ichdr,
+ ent, i, &last_hashval);
+ if (fa)
+ return fa;
+ }
/*
* Quickly check the freemap information. Attribute data has to be
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] xfs: check attribute leaf block structure
2019-10-29 4:03 ` [PATCH 1/4] xfs: check attribute leaf block structure Darrick J. Wong
@ 2019-10-29 10:03 ` Brian Foster
0 siblings, 0 replies; 11+ messages in thread
From: Brian Foster @ 2019-10-29 10:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Mon, Oct 28, 2019 at 09:03:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Add missing structure checks in the attribute leaf verifier.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_attr_leaf.c | 67 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index f0089e862216..bb80a01bcca9 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -232,6 +232,61 @@ xfs_attr3_leaf_hdr_to_disk(
> }
> }
>
> +static xfs_failaddr_t
> +xfs_attr3_leaf_verify_entry(
> + struct xfs_mount *mp,
> + char *buf_end,
> + struct xfs_attr_leafblock *leaf,
> + struct xfs_attr3_icleaf_hdr *leafhdr,
> + struct xfs_attr_leaf_entry *ent,
> + int idx,
> + __u32 *last_hashval)
> +{
> + struct xfs_attr_leaf_name_local *lentry;
> + struct xfs_attr_leaf_name_remote *rentry;
> + char *name_end;
> + unsigned int nameidx;
> + unsigned int namesize;
> + __u32 hashval;
> +
> + /* hash order check */
> + hashval = be32_to_cpu(ent->hashval);
> + if (hashval < *last_hashval)
> + return __this_address;
> + *last_hashval = hashval;
> +
> + nameidx = be16_to_cpu(ent->nameidx);
> + if (nameidx < leafhdr->firstused || nameidx >= mp->m_attr_geo->blksize)
> + return __this_address;
> +
> + /*
> + * Check the name information. The namelen fields are u8 so we can't
> + * possibly exceed the maximum name length of 255 bytes.
> + */
> + if (ent->flags & XFS_ATTR_LOCAL) {
> + lentry = xfs_attr3_leaf_name_local(leaf, idx);
> + namesize = xfs_attr_leaf_entsize_local(lentry->namelen,
> + be16_to_cpu(lentry->valuelen));
> + name_end = (char *)lentry + namesize;
> + if (lentry->namelen == 0)
> + return __this_address;
> + } else {
> + rentry = xfs_attr3_leaf_name_remote(leaf, idx);
> + namesize = xfs_attr_leaf_entsize_remote(rentry->namelen);
> + name_end = (char *)rentry + namesize;
> + if (rentry->namelen == 0)
> + return __this_address;
> + if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
> + rentry->valueblk == 0)
> + return __this_address;
> + }
> +
> + if (name_end > buf_end)
> + return __this_address;
> +
> + return NULL;
> +}
> +
> static xfs_failaddr_t
> xfs_attr3_leaf_verify(
> struct xfs_buf *bp)
> @@ -240,7 +295,10 @@ xfs_attr3_leaf_verify(
> struct xfs_mount *mp = bp->b_mount;
> struct xfs_attr_leafblock *leaf = bp->b_addr;
> struct xfs_attr_leaf_entry *entries;
> + struct xfs_attr_leaf_entry *ent;
> + char *buf_end;
> uint32_t end; /* must be 32bit - see below */
> + __u32 last_hashval = 0;
> int i;
> xfs_failaddr_t fa;
>
> @@ -273,8 +331,13 @@ xfs_attr3_leaf_verify(
> (char *)bp->b_addr + ichdr.firstused)
> return __this_address;
>
> - /* XXX: need to range check rest of attr header values */
> - /* XXX: hash order check? */
> + buf_end = (char *)bp->b_addr + mp->m_attr_geo->blksize;
> + for (i = 0, ent = entries; i < ichdr.count; ent++, i++) {
> + fa = xfs_attr3_leaf_verify_entry(mp, buf_end, leaf, &ichdr,
> + ent, i, &last_hashval);
> + if (fa)
> + return fa;
> + }
>
> /*
> * Quickly check the freemap information. Attribute data has to be
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] xfs: namecheck attribute names before listing them
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 4:03 ` Darrick J. Wong
2019-10-29 10:03 ` Brian Foster
2019-10-29 4:04 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
2019-10-29 4:04 ` [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata Darrick J. Wong
3 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2019-10-29 4:03 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, bfoster, hch
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 | 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] xfs: namecheck attribute names before listing them
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
0 siblings, 0 replies; 11+ messages in thread
From: Brian Foster @ 2019-10-29 10:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] xfs: namecheck directory entry names before listing them
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 4:03 ` [PATCH 2/4] xfs: namecheck attribute names before listing them Darrick J. Wong
@ 2019-10-29 4:04 ` 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
3 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2019-10-29 4:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, bfoster, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Actually call namecheck on directory entry names before we hand them
over to userspace.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_dir2_readdir.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 283df898dd9f..a0bec0931f3b 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -17,6 +17,7 @@
#include "xfs_trace.h"
#include "xfs_bmap.h"
#include "xfs_trans.h"
+#include "xfs_error.h"
/*
* Directory file type support functions
@@ -115,6 +116,11 @@ xfs_dir2_sf_getdents(
ino = dp->d_ops->sf_get_ino(sfp, sfep);
filetype = dp->d_ops->sf_get_ftype(sfep);
ctx->pos = off & 0x7fffffff;
+ if (!xfs_dir2_namecheck(sfep->name, sfep->namelen)) {
+ XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+ dp->i_mount);
+ return -EFSCORRUPTED;
+ }
if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
xfs_dir3_get_dtype(dp->i_mount, filetype)))
return 0;
@@ -208,12 +214,16 @@ xfs_dir2_block_getdents(
/*
* If it didn't fit, set the final offset to here & return.
*/
+ if (!xfs_dir2_namecheck(dep->name, dep->namelen)) {
+ XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+ dp->i_mount);
+ error = -EFSCORRUPTED;
+ goto out_rele;
+ }
if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
be64_to_cpu(dep->inumber),
- xfs_dir3_get_dtype(dp->i_mount, filetype))) {
- xfs_trans_brelse(args->trans, bp);
- return 0;
- }
+ xfs_dir3_get_dtype(dp->i_mount, filetype)))
+ goto out_rele;
}
/*
@@ -222,8 +232,9 @@ xfs_dir2_block_getdents(
*/
ctx->pos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk + 1, 0) &
0x7fffffff;
+out_rele:
xfs_trans_brelse(args->trans, bp);
- return 0;
+ return error;
}
/*
@@ -456,6 +467,12 @@ xfs_dir2_leaf_getdents(
filetype = dp->d_ops->data_get_ftype(dep);
ctx->pos = xfs_dir2_byte_to_dataptr(curoff) & 0x7fffffff;
+ if (!xfs_dir2_namecheck(dep->name, dep->namelen)) {
+ XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+ dp->i_mount);
+ error = -EFSCORRUPTED;
+ break;
+ }
if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
be64_to_cpu(dep->inumber),
xfs_dir3_get_dtype(dp->i_mount, filetype)))
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-29 4:04 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
@ 2019-10-29 10:04 ` Brian Foster
0 siblings, 0 replies; 11+ messages in thread
From: Brian Foster @ 2019-10-29 10:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Mon, Oct 28, 2019 at 09:04:05PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Actually call namecheck on directory entry names before we hand them
> over to userspace.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_dir2_readdir.c | 27 ++++++++++++++++++++++-----
> 1 file changed, 22 insertions(+), 5 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index 283df898dd9f..a0bec0931f3b 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -17,6 +17,7 @@
> #include "xfs_trace.h"
> #include "xfs_bmap.h"
> #include "xfs_trans.h"
> +#include "xfs_error.h"
>
> /*
> * Directory file type support functions
> @@ -115,6 +116,11 @@ xfs_dir2_sf_getdents(
> ino = dp->d_ops->sf_get_ino(sfp, sfep);
> filetype = dp->d_ops->sf_get_ftype(sfep);
> ctx->pos = off & 0x7fffffff;
> + if (!xfs_dir2_namecheck(sfep->name, sfep->namelen)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> + dp->i_mount);
> + return -EFSCORRUPTED;
> + }
> if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
> xfs_dir3_get_dtype(dp->i_mount, filetype)))
> return 0;
> @@ -208,12 +214,16 @@ xfs_dir2_block_getdents(
> /*
> * If it didn't fit, set the final offset to here & return.
> */
> + if (!xfs_dir2_namecheck(dep->name, dep->namelen)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> + dp->i_mount);
> + error = -EFSCORRUPTED;
> + goto out_rele;
> + }
> if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
> be64_to_cpu(dep->inumber),
> - xfs_dir3_get_dtype(dp->i_mount, filetype))) {
> - xfs_trans_brelse(args->trans, bp);
> - return 0;
> - }
> + xfs_dir3_get_dtype(dp->i_mount, filetype)))
> + goto out_rele;
> }
>
> /*
> @@ -222,8 +232,9 @@ xfs_dir2_block_getdents(
> */
> ctx->pos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk + 1, 0) &
> 0x7fffffff;
> +out_rele:
> xfs_trans_brelse(args->trans, bp);
> - return 0;
> + return error;
> }
>
> /*
> @@ -456,6 +467,12 @@ xfs_dir2_leaf_getdents(
> filetype = dp->d_ops->data_get_ftype(dep);
>
> ctx->pos = xfs_dir2_byte_to_dataptr(curoff) & 0x7fffffff;
> + if (!xfs_dir2_namecheck(dep->name, dep->namelen)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> + dp->i_mount);
> + error = -EFSCORRUPTED;
> + break;
> + }
> if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
> be64_to_cpu(dep->inumber),
> xfs_dir3_get_dtype(dp->i_mount, filetype)))
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata
2019-10-29 4:03 [PATCH v2 0/4] xfs: more metadata verifier tightening Darrick J. Wong
` (2 preceding siblings ...)
2019-10-29 4:04 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
@ 2019-10-29 4:04 ` Darrick J. Wong
3 siblings, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2019-10-29 4:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, bfoster, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
There are a few places where we return -EIO instead of -EFSCORRUPTED
when we find corrupt metadata. Fix those places.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 6 +++---
fs/xfs/xfs_attr_inactive.c | 6 +++---
fs/xfs/xfs_dquot.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 02469d59c787..587889585a23 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1374,7 +1374,7 @@ xfs_bmap_last_before(
case XFS_DINODE_FMT_EXTENTS:
break;
default:
- return -EIO;
+ return -EFSCORRUPTED;
}
if (!(ifp->if_flags & XFS_IFEXTENTS)) {
@@ -1475,7 +1475,7 @@ xfs_bmap_last_offset(
if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS)
- return -EIO;
+ return -EFSCORRUPTED;
error = xfs_bmap_last_extent(NULL, ip, whichfork, &rec, &is_empty);
if (error || is_empty)
@@ -5864,7 +5864,7 @@ xfs_bmap_insert_extents(
del_cursor);
if (stop_fsb >= got.br_startoff + got.br_blockcount) {
- error = -EIO;
+ error = -EFSCORRUPTED;
goto del_cursor;
}
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index a640a285cc52..f83f11d929e4 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -209,7 +209,7 @@ xfs_attr3_node_inactive(
*/
if (level > XFS_DA_NODE_MAXDEPTH) {
xfs_trans_brelse(*trans, bp); /* no locks for later trans */
- return -EIO;
+ return -EFSCORRUPTED;
}
node = bp->b_addr;
@@ -258,7 +258,7 @@ xfs_attr3_node_inactive(
error = xfs_attr3_leaf_inactive(trans, dp, child_bp);
break;
default:
- error = -EIO;
+ error = -EFSCORRUPTED;
xfs_trans_brelse(*trans, child_bp);
break;
}
@@ -341,7 +341,7 @@ xfs_attr3_root_inactive(
error = xfs_attr3_leaf_inactive(trans, dp, bp);
break;
default:
- error = -EIO;
+ error = -EFSCORRUPTED;
xfs_trans_brelse(*trans, bp);
break;
}
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index aeb95e7391c1..2b87c96fb2c0 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -1126,7 +1126,7 @@ xfs_qm_dqflush(
xfs_buf_relse(bp);
xfs_dqfunlock(dqp);
xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
- return -EIO;
+ return -EFSCORRUPTED;
}
/* This is the only portion of data that needs to persist */
^ permalink raw reply related [flat|nested] 11+ messages in thread