* [PATCH 1/4] xfs: check attribute leaf block structure
2019-10-25 5:14 [PATCH 0/4] xfs: more metadata verifier tightening Darrick J. Wong
@ 2019-10-25 5:14 ` Darrick J. Wong
2019-10-28 18:18 ` Brian Foster
2019-10-25 5:14 ` [PATCH 2/4] xfs: namecheck attribute names before listing them Darrick J. Wong
` (2 subsequent siblings)
3 siblings, 1 reply; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-25 5:14 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
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 | 63 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index ec7921e07f69..8dea3a273029 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -232,6 +232,57 @@ 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. */
+ 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 (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 +291,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 +327,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] 21+ messages in thread* Re: [PATCH 1/4] xfs: check attribute leaf block structure
2019-10-25 5:14 ` [PATCH 1/4] xfs: check attribute leaf block structure Darrick J. Wong
@ 2019-10-28 18:18 ` Brian Foster
2019-10-28 18:27 ` Darrick J. Wong
0 siblings, 1 reply; 21+ messages in thread
From: Brian Foster @ 2019-10-28 18:18 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Thu, Oct 24, 2019 at 10:14:53PM -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>
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 63 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index ec7921e07f69..8dea3a273029 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -232,6 +232,57 @@ 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. */
> + 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;
I think this reads a little better if we check the lentry value before
we use it (same deal with rentry in the branch below).
Also, why the == 0 checks specifically? Or IOW, might there also be a
sane max value to check some of these fields against?
> + } 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 (rentry->valueblk == 0)
> + return __this_address;
Hmm.. ISTR that it's currently possible to have ->valueblk == 0 on an
incomplete remote attr after a crash. That's not ideal and hopefully
fixed up after the xattr intent stuff lands, but in the meantime I
thought we had code sprinkled around somewhere to fix that up after the
fact. Would this turn that scenario into a metadata I/O error?
Brian
> + }
> +
> + if (name_end > buf_end)
> + return __this_address;
> +
> + return NULL;
> +}
> +
> static xfs_failaddr_t
> xfs_attr3_leaf_verify(
> struct xfs_buf *bp)
> @@ -240,7 +291,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 +327,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] 21+ messages in thread* Re: [PATCH 1/4] xfs: check attribute leaf block structure
2019-10-28 18:18 ` Brian Foster
@ 2019-10-28 18:27 ` Darrick J. Wong
0 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-28 18:27 UTC (permalink / raw)
To: Brian Foster; +Cc: linux-xfs
On Mon, Oct 28, 2019 at 02:18:13PM -0400, Brian Foster wrote:
> On Thu, Oct 24, 2019 at 10:14:53PM -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>
> > ---
> > fs/xfs/libxfs/xfs_attr_leaf.c | 63 ++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 61 insertions(+), 2 deletions(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index ec7921e07f69..8dea3a273029 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -232,6 +232,57 @@ 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. */
> > + 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;
>
> I think this reads a little better if we check the lentry value before
> we use it (same deal with rentry in the branch below).
>
> Also, why the == 0 checks specifically? Or IOW, might there also be a
> sane max value to check some of these fields against?
Attributes have a maximum name length of 255 characters, and the ondisk
namelen field is u8, so it's never possible to exceed the maximum.
> > + } 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 (rentry->valueblk == 0)
> > + return __this_address;
>
> Hmm.. ISTR that it's currently possible to have ->valueblk == 0 on an
> incomplete remote attr after a crash. That's not ideal and hopefully
> fixed up after the xattr intent stuff lands, but in the meantime I
> thought we had code sprinkled around somewhere to fix that up after the
> fact. Would this turn that scenario into a metadata I/O error?
<urk> Yes, it would. I'll fix that.
--D
>
> Brian
>
> > + }
> > +
> > + if (name_end > buf_end)
> > + return __this_address;
> > +
> > + return NULL;
> > +}
> > +
> > static xfs_failaddr_t
> > xfs_attr3_leaf_verify(
> > struct xfs_buf *bp)
> > @@ -240,7 +291,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 +327,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] 21+ messages in thread
* [PATCH 2/4] xfs: namecheck attribute names before listing them
2019-10-25 5:14 [PATCH 0/4] xfs: more metadata verifier tightening Darrick J. Wong
2019-10-25 5:14 ` [PATCH 1/4] xfs: check attribute leaf block structure Darrick J. Wong
@ 2019-10-25 5:14 ` Darrick J. Wong
2019-10-28 18:18 ` Brian Foster
2019-10-25 5:15 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
2019-10-25 5:15 ` [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata Darrick J. Wong
3 siblings, 1 reply; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-25 5:14 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
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/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..3a1158a1347d 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -84,6 +84,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,
@@ -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;
+ }
context->put_listent(context,
sbp->flags,
sbp->name,
@@ -284,7 +294,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 +368,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 +381,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 +429,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 +469,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 +481,7 @@ xfs_attr3_leaf_list_int(
cursor->offset++;
}
trace_xfs_attr_list_leaf_end(context);
- return;
+ return 0;
}
/*
@@ -483,9 +500,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
@@ -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;
+ }
+
/*
* Only list entries in the right namespace.
*/
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/4] xfs: namecheck attribute names before listing them
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
0 siblings, 1 reply; 21+ messages in thread
From: Brian Foster @ 2019-10-28 18:18 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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.
> */
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/4] xfs: namecheck attribute names before listing them
2019-10-28 18:18 ` Brian Foster
@ 2019-10-28 18:22 ` Darrick J. Wong
0 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-28 18:22 UTC (permalink / raw)
To: Brian Foster; +Cc: linux-xfs
On Mon, Oct 28, 2019 at 02:18:57PM -0400, Brian Foster wrote:
> 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.
Fixed. Good catch! :)
> > 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?
Oops. I guess I got overzealous.
--D
> Brian
>
> > /*
> > * Only list entries in the right namespace.
> > */
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-25 5:14 [PATCH 0/4] xfs: more metadata verifier tightening Darrick J. Wong
2019-10-25 5:14 ` [PATCH 1/4] xfs: check attribute leaf block structure Darrick J. Wong
2019-10-25 5:14 ` [PATCH 2/4] xfs: namecheck attribute names before listing them Darrick J. Wong
@ 2019-10-25 5:15 ` Darrick J. Wong
2019-10-25 12:56 ` Christoph Hellwig
2019-10-28 18:19 ` Brian Foster
2019-10-25 5:15 ` [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata Darrick J. Wong
3 siblings, 2 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-25 5:15 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
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>
---
fs/xfs/xfs_dir2_readdir.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 283df898dd9f..a8fb0a6829fd 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,6 +214,11 @@ 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);
+ return -EFSCORRUPTED;
+ }
if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
be64_to_cpu(dep->inumber),
xfs_dir3_get_dtype(dp->i_mount, filetype))) {
@@ -456,6 +467,11 @@ 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);
+ return -EFSCORRUPTED;
+ }
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] 21+ messages in thread* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-25 5:15 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
@ 2019-10-25 12:56 ` Christoph Hellwig
2019-10-25 16:04 ` Darrick J. Wong
2019-10-28 18:19 ` Brian Foster
1 sibling, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2019-10-25 12:56 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Thu, Oct 24, 2019 at 10:15: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.
This looks sensible:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Note that we can remove the check for '/' from xfs_dir2_namecheck for
currentl mainline, given that verify_dirent_name in common code now has
that check.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-25 12:56 ` Christoph Hellwig
@ 2019-10-25 16:04 ` Darrick J. Wong
2019-10-29 7:16 ` Christoph Hellwig
0 siblings, 1 reply; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-25 16:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Oct 25, 2019 at 05:56:28AM -0700, Christoph Hellwig wrote:
> On Thu, Oct 24, 2019 at 10:15: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.
>
> This looks sensible:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Note that we can remove the check for '/' from xfs_dir2_namecheck for
> currentl mainline, given that verify_dirent_name in common code now has
> that check.
We can't, because this is the same function that xfs_repair uses to
decide if a directory entry is garbage.
--D
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-25 16:04 ` Darrick J. Wong
@ 2019-10-29 7:16 ` Christoph Hellwig
2019-10-29 16:23 ` Darrick J. Wong
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2019-10-29 7:16 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Fri, Oct 25, 2019 at 09:04:48AM -0700, Darrick J. Wong wrote:
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> >
> > Note that we can remove the check for '/' from xfs_dir2_namecheck for
> > currentl mainline, given that verify_dirent_name in common code now has
> > that check.
>
> We can't, because this is the same function that xfs_repair uses to
> decide if a directory entry is garbage.
So we'll at least need to document that for now. And maybe find a way
to not do the work twice eventually in a way that doesn't break repair.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-29 7:16 ` Christoph Hellwig
@ 2019-10-29 16:23 ` Darrick J. Wong
2019-10-30 21:32 ` Christoph Hellwig
0 siblings, 1 reply; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-29 16:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Oct 29, 2019 at 12:16:15AM -0700, Christoph Hellwig wrote:
> On Fri, Oct 25, 2019 at 09:04:48AM -0700, Darrick J. Wong wrote:
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > >
> > > Note that we can remove the check for '/' from xfs_dir2_namecheck for
> > > currentl mainline, given that verify_dirent_name in common code now has
> > > that check.
> >
> > We can't, because this is the same function that xfs_repair uses to
> > decide if a directory entry is garbage.
>
> So we'll at least need to document that for now. And maybe find a way
> to not do the work twice eventually in a way that doesn't break repair.
What if we promote EFSCORRUPTED and EFSBADCRC to the vfs (since 5
filesystems use them now); change the VFS check function to return that;
and then we can just drop the xfs readdir calls to dir2_namecheck?
--D
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-29 16:23 ` Darrick J. Wong
@ 2019-10-30 21:32 ` Christoph Hellwig
2019-10-30 22:18 ` Darrick J. Wong
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2019-10-30 21:32 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Tue, Oct 29, 2019 at 09:23:30AM -0700, Darrick J. Wong wrote:
> > So we'll at least need to document that for now. And maybe find a way
> > to not do the work twice eventually in a way that doesn't break repair.
>
> What if we promote EFSCORRUPTED and EFSBADCRC to the vfs (since 5
> filesystems use them now); change the VFS check function to return that;
> and then we can just drop the xfs readdir calls to dir2_namecheck?
EFSCORRUPTED should have moved to common code a long time ago, so that
is overdue. Not sure about EFSBADCRC, but that might not be a horrible
idea either.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-30 21:32 ` Christoph Hellwig
@ 2019-10-30 22:18 ` Darrick J. Wong
0 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-30 22:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, Oct 30, 2019 at 02:32:02PM -0700, Christoph Hellwig wrote:
> On Tue, Oct 29, 2019 at 09:23:30AM -0700, Darrick J. Wong wrote:
> > > So we'll at least need to document that for now. And maybe find a way
> > > to not do the work twice eventually in a way that doesn't break repair.
> >
> > What if we promote EFSCORRUPTED and EFSBADCRC to the vfs (since 5
> > filesystems use them now); change the VFS check function to return that;
> > and then we can just drop the xfs readdir calls to dir2_namecheck?
Having looked more carefully at verify_dirent_name(), I now think XFS
shouldn't drop the xfs_readdir calls to dir2_namecheck because the VFS
namecheck function permits nulls in the middle of the name. Linus says
the function does that intentionally because (in his opinion) userspace
expects a null terminated string and won't care if namelen is longer
than that.
> EFSCORRUPTED should have moved to common code a long time ago, so that
> is overdue. Not sure about EFSBADCRC, but that might not be a horrible
> idea either.
We might as well, since ext4 and XFS both standardized on both of those
error codes.
Hm, btrfs uses EUCLEAN and EIO for EFSCORRUPTED and EFSBADCRC,
respectively. We probably ought to get them on board too.
--D
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-25 5:15 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
2019-10-25 12:56 ` Christoph Hellwig
@ 2019-10-28 18:19 ` Brian Foster
2019-10-28 18:23 ` Darrick J. Wong
1 sibling, 1 reply; 21+ messages in thread
From: Brian Foster @ 2019-10-28 18:19 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Thu, Oct 24, 2019 at 10:15: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>
> ---
> fs/xfs/xfs_dir2_readdir.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index 283df898dd9f..a8fb0a6829fd 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
...
> @@ -208,6 +214,11 @@ 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);
> + return -EFSCORRUPTED;
> + }
xfs_trans_brelse(..., bp) (here and in _leaf_getdents())?
Brian
> if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
> be64_to_cpu(dep->inumber),
> xfs_dir3_get_dtype(dp->i_mount, filetype))) {
> @@ -456,6 +467,11 @@ 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);
> + return -EFSCORRUPTED;
> + }
> 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] 21+ messages in thread* Re: [PATCH 3/4] xfs: namecheck directory entry names before listing them
2019-10-28 18:19 ` Brian Foster
@ 2019-10-28 18:23 ` Darrick J. Wong
0 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-28 18:23 UTC (permalink / raw)
To: Brian Foster; +Cc: linux-xfs
On Mon, Oct 28, 2019 at 02:19:15PM -0400, Brian Foster wrote:
> On Thu, Oct 24, 2019 at 10:15: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>
> > ---
> > fs/xfs/xfs_dir2_readdir.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> >
> > diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> > index 283df898dd9f..a8fb0a6829fd 100644
> > --- a/fs/xfs/xfs_dir2_readdir.c
> > +++ b/fs/xfs/xfs_dir2_readdir.c
> ...
> > @@ -208,6 +214,11 @@ 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);
> > + return -EFSCORRUPTED;
> > + }
>
> xfs_trans_brelse(..., bp) (here and in _leaf_getdents())?
Will fix.
--D
> Brian
>
> > if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
> > be64_to_cpu(dep->inumber),
> > xfs_dir3_get_dtype(dp->i_mount, filetype))) {
> > @@ -456,6 +467,11 @@ 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);
> > + return -EFSCORRUPTED;
> > + }
> > 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] 21+ messages in thread
* [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata
2019-10-25 5:14 [PATCH 0/4] xfs: more metadata verifier tightening Darrick J. Wong
` (2 preceding siblings ...)
2019-10-25 5:15 ` [PATCH 3/4] xfs: namecheck directory entry " Darrick J. Wong
@ 2019-10-25 5:15 ` Darrick J. Wong
2019-10-25 12:54 ` Christoph Hellwig
2019-10-28 18:19 ` Brian Foster
3 siblings, 2 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-10-25 5:15 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
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>
---
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] 21+ messages in thread* Re: [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata
2019-10-25 5:15 ` [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata Darrick J. Wong
@ 2019-10-25 12:54 ` Christoph Hellwig
2019-10-28 18:19 ` Brian Foster
1 sibling, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2019-10-25 12:54 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata
2019-10-25 5:15 ` [PATCH 4/4] xfs: replace -EIO with -EFSCORRUPTED for corrupt metadata Darrick J. Wong
2019-10-25 12:54 ` Christoph Hellwig
@ 2019-10-28 18:19 ` Brian Foster
1 sibling, 0 replies; 21+ messages in thread
From: Brian Foster @ 2019-10-28 18:19 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Thu, Oct 24, 2019 at 10:15:11PM -0700, Darrick J. Wong wrote:
> 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: 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 [flat|nested] 21+ messages in thread