From: Brian Foster Date: Mon, 6 May 2019 22:00:28 +0000 (-0400) Subject: xfs: distinguish between inobt and finobt magic values X-Git-Tag: v5.1.0-rc0~23 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c08793bd924532d6395d555dd4f0edc663eb6d5c;p=thirdparty%2Fxfsprogs-dev.git 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 Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong Signed-off-by: Eric Sandeen --- diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h index 5bcf7331c..569516c45 100644 --- 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 b45d07eb2..a2a83884b 100644 --- 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 a5efc805a..450fed013 100644 --- 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 2455ab68d..84acf1682 100644 --- 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,