| 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
|
| Previous by Date: | Re: Performance impact of mkfs.xfs vs mkfs.xfs -f, Carlos E. R. |
|---|---|
| Next by Date: | Re: [PATCH 07/11] xfs_repair: force not-so-bad bmbt blocks back through the verifier, Dave Chinner |
| Previous by Thread: | [PATCH 06/11] xfs_repair: check v5 filesystem attr block header sanity, Darrick J. Wong |
| Next by Thread: | Re: [PATCH 06/11] xfs_repair: check v5 filesystem attr block header sanity, Darrick J. Wong |
| Indexes: | [Date] [Thread] [Top] [All Lists] |