* [PATCH] xfs: enhance dinode verifier
@ 2018-04-13 2:49 Eric Sandeen
2018-04-13 2:55 ` [PATCH V2] " Eric Sandeen
0 siblings, 1 reply; 7+ messages in thread
From: Eric Sandeen @ 2018-04-13 2:49 UTC (permalink / raw)
To: linux-xfs; +Cc: wen.xu
Add several more validations to xfs_dinode_verify:
- For LOCAL data fork formats, di_nextents must be 0.
- For LOCAL attr fork formats, di_anextents must be 0.
- For inodes with no attr fork offset,
- format must be XFS_DINODE_FMT_EXTENTS if set at all
- di_anextents must be 0.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 4fe17b368316..72ab4d2df8cf 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -458,6 +458,8 @@ xfs_dinode_verify(
return __this_address;
if (di_size > XFS_DFORK_DSIZE(dip, mp))
return __this_address;
+ if (dip->di_nextents)
+ return __this_address;
/* fall through */
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
@@ -476,12 +478,25 @@ xfs_dinode_verify(
if (XFS_DFORK_Q(dip)) {
switch (dip->di_aformat) {
case XFS_DINODE_FMT_LOCAL:
+ if (dip->di_anextents)
+ return __this_address;
+ /* fall through */
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
break;
default:
return __this_address;
}
+ } else { /* No attr fork */
+ switch (dip->di_aformat) {
+ case 0: /* Uninitialized OK */
+ case XFS_DINODE_FMT_EXTENTS:
+ break;
+ default:
+ return __this_address;
+ }
+ if (dip->di_anextents)
+ return __this_address;
}
/* only version 3 or greater inodes are extensively verified here */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V2] xfs: enhance dinode verifier
2018-04-13 2:49 [PATCH] xfs: enhance dinode verifier Eric Sandeen
@ 2018-04-13 2:55 ` Eric Sandeen
2018-04-13 3:56 ` Darrick J. Wong
2018-04-13 15:54 ` [PATCH V3] " Eric Sandeen
0 siblings, 2 replies; 7+ messages in thread
From: Eric Sandeen @ 2018-04-13 2:55 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs; +Cc: wen.xu
Add several more validations to xfs_dinode_verify:
- For LOCAL data fork formats, di_nextents must be 0.
- For LOCAL attr fork formats, di_anextents must be 0.
- For inodes with no attr fork offset,
- format must be XFS_DINODE_FMT_EXTENTS if set at all
- di_anextents must be 0.
Thanks to dchinner for pointing out a couple related checks I had
forgotten to add.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: due credit :)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 4fe17b368316..72ab4d2df8cf 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -458,6 +458,8 @@ xfs_dinode_verify(
return __this_address;
if (di_size > XFS_DFORK_DSIZE(dip, mp))
return __this_address;
+ if (dip->di_nextents)
+ return __this_address;
/* fall through */
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
@@ -476,12 +478,25 @@ xfs_dinode_verify(
if (XFS_DFORK_Q(dip)) {
switch (dip->di_aformat) {
case XFS_DINODE_FMT_LOCAL:
+ if (dip->di_anextents)
+ return __this_address;
+ /* fall through */
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
break;
default:
return __this_address;
}
+ } else { /* No attr fork */
+ switch (dip->di_aformat) {
+ case 0: /* Uninitialized OK */
+ case XFS_DINODE_FMT_EXTENTS:
+ break;
+ default:
+ return __this_address;
+ }
+ if (dip->di_anextents)
+ return __this_address;
}
/* only version 3 or greater inodes are extensively verified here */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2] xfs: enhance dinode verifier
2018-04-13 2:55 ` [PATCH V2] " Eric Sandeen
@ 2018-04-13 3:56 ` Darrick J. Wong
2018-04-13 3:58 ` Eric Sandeen
2018-04-13 15:54 ` [PATCH V3] " Eric Sandeen
1 sibling, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2018-04-13 3:56 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs, wen.xu
On Thu, Apr 12, 2018 at 09:55:41PM -0500, Eric Sandeen wrote:
> Add several more validations to xfs_dinode_verify:
>
> - For LOCAL data fork formats, di_nextents must be 0.
> - For LOCAL attr fork formats, di_anextents must be 0.
> - For inodes with no attr fork offset,
> - format must be XFS_DINODE_FMT_EXTENTS if set at all
> - di_anextents must be 0.
>
> Thanks to dchinner for pointing out a couple related checks I had
> forgotten to add.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> V2: due credit :)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 4fe17b368316..72ab4d2df8cf 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -458,6 +458,8 @@ xfs_dinode_verify(
> return __this_address;
> if (di_size > XFS_DFORK_DSIZE(dip, mp))
> return __this_address;
> + if (dip->di_nextents)
> + return __this_address;
> /* fall through */
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> @@ -476,12 +478,25 @@ xfs_dinode_verify(
> if (XFS_DFORK_Q(dip)) {
> switch (dip->di_aformat) {
> case XFS_DINODE_FMT_LOCAL:
> + if (dip->di_anextents)
> + return __this_address;
> + /* fall through */
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> break;
> default:
> return __this_address;
> }
> + } else { /* No attr fork */
> + switch (dip->di_aformat) {
> + case 0: /* Uninitialized OK */
Might be a good idea to point out that a newly allocated inode chunk
will have all these fields set to zero (as I apparently stumble over
this about once every 10-11 months).
Also, uh, does xfs_repair catch these? :D
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> + case XFS_DINODE_FMT_EXTENTS:
> + break;
> + default:
> + return __this_address;
> + }
> + if (dip->di_anextents)
> + return __this_address;
> }
>
> /* only version 3 or greater inodes are extensively verified here */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] xfs: enhance dinode verifier
2018-04-13 3:56 ` Darrick J. Wong
@ 2018-04-13 3:58 ` Eric Sandeen
0 siblings, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2018-04-13 3:58 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Eric Sandeen, linux-xfs, wen.xu
On 4/12/18 10:56 PM, Darrick J. Wong wrote:
> On Thu, Apr 12, 2018 at 09:55:41PM -0500, Eric Sandeen wrote:
>> Add several more validations to xfs_dinode_verify:
...
>> @@ -476,12 +478,25 @@ xfs_dinode_verify(
>> if (XFS_DFORK_Q(dip)) {
>> switch (dip->di_aformat) {
>> case XFS_DINODE_FMT_LOCAL:
>> + if (dip->di_anextents)
>> + return __this_address;
>> + /* fall through */
>> case XFS_DINODE_FMT_EXTENTS:
>> case XFS_DINODE_FMT_BTREE:
>> break;
>> default:
>> return __this_address;
>> }
>> + } else { /* No attr fork */
>> + switch (dip->di_aformat) {
>> + case 0: /* Uninitialized OK */
>
> Might be a good idea to point out that a newly allocated inode chunk
> will have all these fields set to zero
Well, that was the intent of my terse comment... I can write a longer
one, clearly it did not do the trick?
-Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V3] xfs: enhance dinode verifier
2018-04-13 2:55 ` [PATCH V2] " Eric Sandeen
2018-04-13 3:56 ` Darrick J. Wong
@ 2018-04-13 15:54 ` Eric Sandeen
2018-04-13 15:59 ` Darrick J. Wong
2018-04-17 4:35 ` Eric Sandeen
1 sibling, 2 replies; 7+ messages in thread
From: Eric Sandeen @ 2018-04-13 15:54 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs; +Cc: wen.xu
Add several more validations to xfs_dinode_verify:
- For LOCAL data fork formats, di_nextents must be 0.
- For LOCAL attr fork formats, di_anextents must be 0.
- For inodes with no attr fork offset,
- format must be XFS_DINODE_FMT_EXTENTS if set at all
- di_anextents must be 0.
Thanks to dchinner for pointing out a couple related checks I had
forgotten to add.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: due credit
V3: clarify the no-offset / no-format cases in a comment
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index ef68b1d..ee8ed90 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -466,6 +466,8 @@
return __this_address;
if (di_size > XFS_DFORK_DSIZE(dip, mp))
return __this_address;
+ if (dip->di_nextents)
+ return __this_address;
/* fall through */
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
@@ -484,12 +486,31 @@
if (XFS_DFORK_Q(dip)) {
switch (dip->di_aformat) {
case XFS_DINODE_FMT_LOCAL:
+ if (dip->di_anextents)
+ return __this_address;
+ /* fall through */
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
break;
default:
return __this_address;
}
+ } else {
+ /*
+ * If there is no fork offset, this may be a freshly-made inode
+ * in a new disk cluster, in which case di_aformat is zeroed.
+ * Otherwise, such an inode must be in EXTENTS format; this goes
+ * for freed inodes as well.
+ */
+ switch (dip->di_aformat) {
+ case 0:
+ case XFS_DINODE_FMT_EXTENTS:
+ break;
+ default:
+ return __this_address;
+ }
+ if (dip->di_anextents)
+ return __this_address;
}
/* only version 3 or greater inodes are extensively verified here */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V3] xfs: enhance dinode verifier
2018-04-13 15:54 ` [PATCH V3] " Eric Sandeen
@ 2018-04-13 15:59 ` Darrick J. Wong
2018-04-17 4:35 ` Eric Sandeen
1 sibling, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2018-04-13 15:59 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs, wen.xu
On Fri, Apr 13, 2018 at 10:54:14AM -0500, Eric Sandeen wrote:
> Add several more validations to xfs_dinode_verify:
>
> - For LOCAL data fork formats, di_nextents must be 0.
> - For LOCAL attr fork formats, di_anextents must be 0.
> - For inodes with no attr fork offset,
> - format must be XFS_DINODE_FMT_EXTENTS if set at all
> - di_anextents must be 0.
>
> Thanks to dchinner for pointing out a couple related checks I had
> forgotten to add.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok, will try to figure out what the online repair ramifications
are...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> V2: due credit
> V3: clarify the no-offset / no-format cases in a comment
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index ef68b1d..ee8ed90 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -466,6 +466,8 @@
> return __this_address;
> if (di_size > XFS_DFORK_DSIZE(dip, mp))
> return __this_address;
> + if (dip->di_nextents)
> + return __this_address;
> /* fall through */
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> @@ -484,12 +486,31 @@
> if (XFS_DFORK_Q(dip)) {
> switch (dip->di_aformat) {
> case XFS_DINODE_FMT_LOCAL:
> + if (dip->di_anextents)
> + return __this_address;
> + /* fall through */
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> break;
> default:
> return __this_address;
> }
> + } else {
> + /*
> + * If there is no fork offset, this may be a freshly-made inode
> + * in a new disk cluster, in which case di_aformat is zeroed.
> + * Otherwise, such an inode must be in EXTENTS format; this goes
> + * for freed inodes as well.
> + */
> + switch (dip->di_aformat) {
> + case 0:
> + case XFS_DINODE_FMT_EXTENTS:
> + break;
> + default:
> + return __this_address;
> + }
> + if (dip->di_anextents)
> + return __this_address;
> }
>
> /* only version 3 or greater inodes are extensively verified here */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] xfs: enhance dinode verifier
2018-04-13 15:54 ` [PATCH V3] " Eric Sandeen
2018-04-13 15:59 ` Darrick J. Wong
@ 2018-04-17 4:35 ` Eric Sandeen
1 sibling, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2018-04-17 4:35 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs; +Cc: wen.xu
On 4/13/18 10:54 AM, Eric Sandeen wrote:
> Add several more validations to xfs_dinode_verify:
>
> - For LOCAL data fork formats, di_nextents must be 0.
> - For LOCAL attr fork formats, di_anextents must be 0.
> - For inodes with no attr fork offset,
> - format must be XFS_DINODE_FMT_EXTENTS if set at all
> - di_anextents must be 0.
>
> Thanks to dchinner for pointing out a couple related checks I had
> forgotten to add.
Darrick, maybe also add:
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199377
If you still can
-Eric
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> V2: due credit
> V3: clarify the no-offset / no-format cases in a comment
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index ef68b1d..ee8ed90 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -466,6 +466,8 @@
> return __this_address;
> if (di_size > XFS_DFORK_DSIZE(dip, mp))
> return __this_address;
> + if (dip->di_nextents)
> + return __this_address;
> /* fall through */
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> @@ -484,12 +486,31 @@
> if (XFS_DFORK_Q(dip)) {
> switch (dip->di_aformat) {
> case XFS_DINODE_FMT_LOCAL:
> + if (dip->di_anextents)
> + return __this_address;
> + /* fall through */
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> break;
> default:
> return __this_address;
> }
> + } else {
> + /*
> + * If there is no fork offset, this may be a freshly-made inode
> + * in a new disk cluster, in which case di_aformat is zeroed.
> + * Otherwise, such an inode must be in EXTENTS format; this goes
> + * for freed inodes as well.
> + */
> + switch (dip->di_aformat) {
> + case 0:
> + case XFS_DINODE_FMT_EXTENTS:
> + break;
> + default:
> + return __this_address;
> + }
> + if (dip->di_anextents)
> + return __this_address;
> }
>
> /* only version 3 or greater inodes are extensively verified here */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-04-17 4:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-13 2:49 [PATCH] xfs: enhance dinode verifier Eric Sandeen
2018-04-13 2:55 ` [PATCH V2] " Eric Sandeen
2018-04-13 3:56 ` Darrick J. Wong
2018-04-13 3:58 ` Eric Sandeen
2018-04-13 15:54 ` [PATCH V3] " Eric Sandeen
2018-04-13 15:59 ` Darrick J. Wong
2018-04-17 4:35 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.