Skip to content

Commit

Permalink
xfs: skip verification on initial "guess" superblock read
Browse files Browse the repository at this point in the history
When xfs_readsb() does the very first read of the superblock,
it makes a guess at the length of the buffer, based on the
sector size of the underlying storage.  This may or may
not match the filesystem sector size in sb_sectsize, so
we can't i.e. do a CRC check on it; it might be too short.

In fact, mounting a filesystem with sb_sectsize larger
than the device sector size will cause a mount failure
if CRCs are enabled, because we are checksumming a length
which exceeds the buffer passed to it.

So always read twice; the first time we read with NULL
buffer ops to skip verification; then set the proper
read length, hook up the proper verifier, and give it
another go.

Once we are sure that we've got the right buffer length,
we can also use bp->b_length in the xfs_sb_read_verify,
rather than the less-trusted on-disk sectorsize for
secondary superblocks.  Before this we ran the risk of
passing junk to the crc32c routines, which didn't always
handle extreme values.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
  • Loading branch information
Eric Sandeen authored and Dave Chinner committed Feb 19, 2014
1 parent 82daa86 commit daba542
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
24 changes: 16 additions & 8 deletions fs/xfs/xfs_mount.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,22 +282,29 @@ xfs_readsb(
struct xfs_sb *sbp = &mp->m_sb;
int error;
int loud = !(flags & XFS_MFSI_QUIET);
const struct xfs_buf_ops *buf_ops;

ASSERT(mp->m_sb_bp == NULL);
ASSERT(mp->m_ddev_targp != NULL);

/*
* For the initial read, we must guess at the sector
* size based on the block device. It's enough to
* get the sb_sectsize out of the superblock and
* then reread with the proper length.
* We don't verify it yet, because it may not be complete.
*/
sector_size = xfs_getsize_buftarg(mp->m_ddev_targp);
buf_ops = NULL;

/*
* Allocate a (locked) buffer to hold the superblock.
* This will be kept around at all times to optimize
* access to the superblock.
*/
sector_size = xfs_getsize_buftarg(mp->m_ddev_targp);

reread:
bp = xfs_buf_read_uncached(mp->m_ddev_targp, XFS_SB_DADDR,
BTOBB(sector_size), 0,
loud ? &xfs_sb_buf_ops
: &xfs_sb_quiet_buf_ops);
BTOBB(sector_size), 0, buf_ops);
if (!bp) {
if (loud)
xfs_warn(mp, "SB buffer read failed");
Expand Down Expand Up @@ -328,12 +335,13 @@ xfs_readsb(
}

/*
* If device sector size is smaller than the superblock size,
* re-read the superblock so the buffer is correctly sized.
* Re-read the superblock so the buffer is correctly sized,
* and properly verified.
*/
if (sector_size < sbp->sb_sectsize) {
if (buf_ops == NULL) {
xfs_buf_relse(bp);
sector_size = sbp->sb_sectsize;
buf_ops = loud ? &xfs_sb_buf_ops : &xfs_sb_quiet_buf_ops;
goto reread;
}

Expand Down
3 changes: 1 addition & 2 deletions fs/xfs/xfs_sb.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ xfs_sb_read_verify(
XFS_SB_VERSION_5) ||
dsb->sb_crc != 0)) {

if (!xfs_verify_cksum(bp->b_addr, be16_to_cpu(dsb->sb_sectsize),
if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
offsetof(struct xfs_sb, sb_crc))) {
/* Only fail bad secondaries on a known V5 filesystem */
if (bp->b_bn == XFS_SB_DADDR ||
Expand Down Expand Up @@ -644,7 +644,6 @@ xfs_sb_quiet_read_verify(
{
struct xfs_dsb *dsb = XFS_BUF_TO_SBP(bp);


if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
/* XFS filesystem, verify noisily! */
xfs_sb_read_verify(bp);
Expand Down

0 comments on commit daba542

Please sign in to comment.