[Top] [All Lists]

[PATCH V3] xfs: skip verification on initial "guess" superblock read

To: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Subject: [PATCH V3] xfs: skip verification on initial "guess" superblock read
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 13 Feb 2014 17:32:12 -0600
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <52FD3C59.3050400@xxxxxxxxxx>
References: <52FD3C59.3050400@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.3.0
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.

Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>

V3: Go back to always reading twice, to avoid the need for
duplicating the code that handles verifier errors.

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 02df7b4..f96c056 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -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 we can 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);
        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");
@@ -328,12 +335,13 @@ reread:
-        * 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) {
                sector_size = sbp->sb_sectsize;
+               buf_ops = loud ? &xfs_sb_buf_ops : &xfs_sb_quiet_buf_ops;
                goto reread;
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 5071ccb..1b0b503 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -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! */

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