xfs: distinguish between inobt and finobt magic values

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>

1 files changed, 7 insertions, 9 deletions

diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 798269eb4767..c2df1f89eec8 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -260,6 +260,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.
@@ -270,18 +273,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 */
@@ -328,6 +323,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,
@@ -335,6 +331,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,