xfs
[Top] [All Lists]

Re: [PATCH 06/11] xfs_repair: check v5 filesystem attr block header sani

To: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Subject: Re: [PATCH 06/11] xfs_repair: check v5 filesystem attr block header sanity
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 26 Aug 2015 10:45:02 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150826003259.23973.34038.stgit@xxxxxxxxxxxxxxxx>
References: <20150826003220.23973.59731.stgit@xxxxxxxxxxxxxxxx> <20150826003259.23973.34038.stgit@xxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Aug 25, 2015 at 05:32:59PM -0700, Darrick J. Wong wrote:
> Check the v5 fields (uuid, blocknr, owner) of attribute blocks for
> obvious errors while scanning xattr blocks.  If the ownership info
> is incorrect, kill the block.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

Why hasn't the buffer verifier done this validation?

> @@ -1564,6 +1602,13 @@ process_longform_attr(
>       if (bp->b_error == -EFSBADCRC)
>               (*repair)++;
>  
> +     /* is this block sane? */
> +     if (__check_attr_header(mp, bp, ino)) {
> +             *repair = 0;
> +             libxfs_putbuf(bp);
> +             return 1;
> +     }

As you can see the above hunk has a bad CRC check from the verifier,
and if the attr header is wrong then the verifier should be setting
bp->b_error == -EFSCORRUPTED.

So shouldn't this simply be:

+       if (bp->b_error == -EFSCORRUPTED) {
+               *repair = 0;
+               libxfs_putbuf(bp);
+               return 1;
+       }
+

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>