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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs