xfs: distinguish between inobt and finobt magic values

Source kernel commit: 8473fee340e37711b9ac6a5cc591305ccaaa4778

The inode btree verifier code is shared between the inode btree and
free inode btree because the underlying metadata formats are
essentially equivalent. A side effect of this is that the verifier
cannot determine whether a particular btree block should have an
inobt or finobt magic value.

This logic allows an unfortunate xfs_repair bug to escape detection
where certain level > 0 nodes of the finobt are stamped with inobt
magic by xfs_repair finobt reconstruction. This is fortunately not a
severe problem since the inode btree magic values do not contribute
to any changes in kernel behavior, but we do need a means to detect
and prevent this problem in the future.

Add a field to xfs_buf_ops to store the v4 and v5 superblock magic
values expected by a particular verifier. Add a helper to check an
on-disk magic value against the value expected by the verifier. Call
the helper from the shared [f]inobt verifier code for magic value
verification. This ensures that the inode btree blocks each have the
appropriate magic value based on specific tree type and superblock
version.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 5bcf7331c7de2ab4ce61e27c8d32b6b898e4c3df..569516c459a09fb4ac666e7276152b64da2198f4 100644 (file)
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -41,6 +41,7 @@ struct xfs_buf_map {
 
 struct xfs_buf_ops {
        char *name;
+       uint32_t magic[2];      /* v4 and v5 on disk magic values */
        void (*verify_read)(struct xfs_buf *);
        void (*verify_write)(struct xfs_buf *);
        xfs_failaddr_t (*verify_struct)(struct xfs_buf *);
@@ -74,6 +75,8 @@ typedef struct xfs_buf {
 #endif
 } xfs_buf_t;
 
+bool xfs_verify_magic(struct xfs_buf *bp, uint32_t dmagic);
+
 enum xfs_buf_flags_t { /* b_flags bits */
        LIBXFS_B_EXIT           = 0x0001,       /* ==LIBXFS_EXIT_ON_FAILURE */
        LIBXFS_B_DIRTY          = 0x0002,       /* buffer has been modified */

diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index b45d07eb2e510c7ea3ce98e93d8c49869b1b76fd..a2a83884b68b98274340d12ceba77de818f2d28f 100644 (file)
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -205,10 +205,11 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 #define rcu_read_lock()                ((void) 0)
 #define rcu_read_unlock()      ((void) 0)
 /* Need to be able to handle this bare or in control flow */
-static inline bool WARN_ON_ONCE(bool expr) {
+static inline bool WARN_ON(bool expr) {
        return (expr);
 }
 
+#define WARN_ON_ONCE(e)                        WARN_ON(e)
 #define percpu_counter_read(x)         (*x)
 #define percpu_counter_read_positive(x)        ((*x) > 0 ? (*x) : 0)
 #define percpu_counter_sum(x)          (*x)

diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index a5efc805a5f641ec1de431640394ef583ae5b8cc..450fed013037e7775e19955b6365ed47f6321526 100644 (file)
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1324,6 +1324,24 @@ struct cache_operations libxfs_bcache_operations = {
        .bulkrelse      = libxfs_bulkrelse
 };
 
+/*
+ * Verify an on-disk magic value against the magic value specified in the
+ * verifier structure. The verifier magic is in disk byte order so the caller is
+ * expected to pass the value directly from disk.
+ */
+bool
+xfs_verify_magic(
+       struct xfs_buf          *bp,
+       uint32_t                dmagic)
+{
+       struct xfs_mount        *mp = bp->b_target->bt_mount;
+       int                     idx;
+
+       idx = xfs_sb_version_hascrc(&mp->m_sb);
+       if (unlikely(WARN_ON(!bp->b_ops || !bp->b_ops->magic[idx])))
+               return false;
+       return dmagic == bp->b_ops->magic[idx];
+}
 
 /*
  * Inode cache stubs.

diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index 2455ab68d6ab60ae9efbb405905899c89a03ea2b..84acf16828657f3a36539098e19d088628ded0b5 100644 (file)
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -259,6 +259,9 @@ xfs_inobt_verify(
        xfs_failaddr_t          fa;
        unsigned int            level;
 
+       if (!xfs_verify_magic(bp, block->bb_magic))
+               return __this_address;
+
        /*
         * During growfs operations, we can't verify the exact owner as the
         * perag is not fully initialised and hence not attached to the buffer.
@@ -269,18 +272,10 @@ xfs_inobt_verify(
         * but beware of the landmine (i.e. need to check pag->pagi_init) if we
         * ever do.
         */
-       switch (block->bb_magic) {
-       case cpu_to_be32(XFS_IBT_CRC_MAGIC):
-       case cpu_to_be32(XFS_FIBT_CRC_MAGIC):
+       if (xfs_sb_version_hascrc(&mp->m_sb)) {
                fa = xfs_btree_sblock_v5hdr_verify(bp);
                if (fa)
                        return fa;
-               /* fall through */
-       case cpu_to_be32(XFS_IBT_MAGIC):
-       case cpu_to_be32(XFS_FIBT_MAGIC):
-               break;
-       default:
-               return __this_address;
        }
 
        /* level verification */
@@ -327,6 +322,7 @@ xfs_inobt_write_verify(
 
 const struct xfs_buf_ops xfs_inobt_buf_ops = {
        .name = "xfs_inobt",
+       .magic = { cpu_to_be32(XFS_IBT_MAGIC), cpu_to_be32(XFS_IBT_CRC_MAGIC) },
        .verify_read = xfs_inobt_read_verify,
        .verify_write = xfs_inobt_write_verify,
        .verify_struct = xfs_inobt_verify,
@@ -334,6 +330,8 @@ const struct xfs_buf_ops xfs_inobt_buf_ops = {
 
 const struct xfs_buf_ops xfs_finobt_buf_ops = {
        .name = "xfs_finobt",
+       .magic = { cpu_to_be32(XFS_FIBT_MAGIC),
+                  cpu_to_be32(XFS_FIBT_CRC_MAGIC) },
        .verify_read = xfs_inobt_read_verify,
        .verify_write = xfs_inobt_write_verify,
        .verify_struct = xfs_inobt_verify,