Commit 8473fee3 authored by Brian Foster's avatar Brian Foster Committed by Darrick J. Wong

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: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 01e68f40
...@@ -260,6 +260,9 @@ xfs_inobt_verify( ...@@ -260,6 +260,9 @@ xfs_inobt_verify(
xfs_failaddr_t fa; xfs_failaddr_t fa;
unsigned int level; 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 * During growfs operations, we can't verify the exact owner as the
* perag is not fully initialised and hence not attached to the buffer. * perag is not fully initialised and hence not attached to the buffer.
...@@ -270,18 +273,10 @@ xfs_inobt_verify( ...@@ -270,18 +273,10 @@ xfs_inobt_verify(
* but beware of the landmine (i.e. need to check pag->pagi_init) if we * but beware of the landmine (i.e. need to check pag->pagi_init) if we
* ever do. * ever do.
*/ */
switch (block->bb_magic) { if (xfs_sb_version_hascrc(&mp->m_sb)) {
case cpu_to_be32(XFS_IBT_CRC_MAGIC):
case cpu_to_be32(XFS_FIBT_CRC_MAGIC):
fa = xfs_btree_sblock_v5hdr_verify(bp); fa = xfs_btree_sblock_v5hdr_verify(bp);
if (fa) if (fa)
return 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 */ /* level verification */
...@@ -328,6 +323,7 @@ xfs_inobt_write_verify( ...@@ -328,6 +323,7 @@ xfs_inobt_write_verify(
const struct xfs_buf_ops xfs_inobt_buf_ops = { const struct xfs_buf_ops xfs_inobt_buf_ops = {
.name = "xfs_inobt", .name = "xfs_inobt",
.magic = { cpu_to_be32(XFS_IBT_MAGIC), cpu_to_be32(XFS_IBT_CRC_MAGIC) },
.verify_read = xfs_inobt_read_verify, .verify_read = xfs_inobt_read_verify,
.verify_write = xfs_inobt_write_verify, .verify_write = xfs_inobt_write_verify,
.verify_struct = xfs_inobt_verify, .verify_struct = xfs_inobt_verify,
...@@ -335,6 +331,8 @@ const struct xfs_buf_ops xfs_inobt_buf_ops = { ...@@ -335,6 +331,8 @@ const struct xfs_buf_ops xfs_inobt_buf_ops = {
const struct xfs_buf_ops xfs_finobt_buf_ops = { const struct xfs_buf_ops xfs_finobt_buf_ops = {
.name = "xfs_finobt", .name = "xfs_finobt",
.magic = { cpu_to_be32(XFS_FIBT_MAGIC),
cpu_to_be32(XFS_FIBT_CRC_MAGIC) },
.verify_read = xfs_inobt_read_verify, .verify_read = xfs_inobt_read_verify,
.verify_write = xfs_inobt_write_verify, .verify_write = xfs_inobt_write_verify,
.verify_struct = xfs_inobt_verify, .verify_struct = xfs_inobt_verify,
......
...@@ -2204,3 +2204,22 @@ void xfs_buf_set_ref(struct xfs_buf *bp, int lru_ref) ...@@ -2204,3 +2204,22 @@ void xfs_buf_set_ref(struct xfs_buf *bp, int lru_ref)
atomic_set(&bp->b_lru_ref, lru_ref); atomic_set(&bp->b_lru_ref, lru_ref);
} }
/*
* 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];
}
...@@ -125,6 +125,7 @@ struct xfs_buf_map { ...@@ -125,6 +125,7 @@ struct xfs_buf_map {
struct xfs_buf_ops { struct xfs_buf_ops {
char *name; char *name;
uint32_t magic[2]; /* v4 and v5 on disk magic values */
void (*verify_read)(struct xfs_buf *); void (*verify_read)(struct xfs_buf *);
void (*verify_write)(struct xfs_buf *); void (*verify_write)(struct xfs_buf *);
xfs_failaddr_t (*verify_struct)(struct xfs_buf *bp); xfs_failaddr_t (*verify_struct)(struct xfs_buf *bp);
...@@ -386,5 +387,6 @@ extern int xfs_setsize_buftarg(xfs_buftarg_t *, unsigned int); ...@@ -386,5 +387,6 @@ extern int xfs_setsize_buftarg(xfs_buftarg_t *, unsigned int);
#define xfs_readonly_buftarg(buftarg) bdev_read_only((buftarg)->bt_bdev) #define xfs_readonly_buftarg(buftarg) bdev_read_only((buftarg)->bt_bdev)
int xfs_buf_reverify(struct xfs_buf *bp, const struct xfs_buf_ops *ops); int xfs_buf_reverify(struct xfs_buf *bp, const struct xfs_buf_ops *ops);
bool xfs_verify_magic(struct xfs_buf *bp, uint32_t dmagic);
#endif /* __XFS_BUF_H__ */ #endif /* __XFS_BUF_H__ */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment