xfs
[Top] [All Lists]

Re: [PATCH, RFC] xfs: don't verify checksum on non-V5 superblocks

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH, RFC] xfs: don't verify checksum on non-V5 superblocks
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 16 Aug 2013 07:00:18 +1000
Cc: "'linux-xfs@xxxxxxxxxxx'" <linux-xfs@xxxxxxxxxxx>
Delivered-to: linux-xfs@xxxxxxxxxxx
In-reply-to: <520D1BA3.1050500@xxxxxxxxxx>
References: <520D1BA3.1050500@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Aug 15, 2013 at 01:19:15PM -0500, Eric Sandeen wrote:
> The current test in xfs_sb_read_verify() will attempt to validate
> an sb checksum if sb_crc is non-zero, even if the superblock is not
> marked as being version 5.
> 
> This runs the risk of picking up random garbage in sb_crc for non-V5
> superblocks; such garbage is known to exist in the wild due to prior bugs.
> This will cause verification to fail for otherwise non-fatal reasons.
> 
> I'm not sure of the point of trying to validate a non-V5 superblock;
> is there one?  Shouldn't this || be an &&?  (Can sb_crc validly be
> 0 for a V5 SB?)

I don't think so.

As I mentioned on the call, the reason for this check is that if we
have a CRC set and a non-v5 superblock version, we may have a
corrupt superblock with bit errors in it. In this case, we check the
CRC to determine if the superblock is intact. If the CRC validates,
then it means that we wrote a bad superblock to disk (i.e. a code
bug). If it doesn't validate, then the superblock is in a corrupt
state because all fields not understood by the v4 superblock should
be zero.

That's why if the checksum fails we are returning EFSCORRUPTED.

The problem we see here is not the validation of the primary
superblock - it's the secondary superblocks that have been written
by growfs that are the problem. We already know that we are
verifying a secondary superblock by the "check_inprogress"
parameter. Hence if we get this problem on a secondary superblock we
can verify it against the primary superblock via the struct
xfs_mount (i.e. mp->m_sb.sb_versionnum) and determine whether we do
indeed have a v4 or v5 superblock and hence determine whether we
should error out or just warn about it.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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