On Mon, Feb 04, 2019 at 09:52:28AM -0500, Brian Foster wrote:
> The allocation btree verifiers share code that is unable to detect
> cross-tree magic value corruptions such as a bnobt block with a
> cntbt magic value. Populate the b_ops->magic field of the associated
> verifier structures such that the structure verifier can check the
> magic value against the expected value based on tree type.
>
> The btree level check requires knowledge of the tree type to
> determine the appropriate maximum value. This was previously part of
> the hardcoded magic value checks. With that code removed, peek at
> the first magic value in the verifier to determine the expected tree
> type of the current block.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/libxfs/xfs_alloc_btree.c | 60 ++++++++++++++-------------------
> 1 file changed, 25 insertions(+), 35 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
> index 480d0f52da64..9fe949f6055e 100644
> --- a/fs/xfs/libxfs/xfs_alloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_alloc_btree.c
> @@ -297,48 +297,34 @@ xfs_allocbt_verify(
> struct xfs_perag *pag = bp->b_pag;
> xfs_failaddr_t fa;
> unsigned int level;
> + xfs_btnum_t btnum = XFS_BTNUM_BNOi;
> +
> + if (!xfs_verify_magic(bp, block->bb_magic))
> + return __this_address;
> +
> + if (xfs_sb_version_hascrc(&mp->m_sb)) {
> + fa = xfs_btree_sblock_v5hdr_verify(bp);
> + if (fa)
> + return fa;
> + }
>
> /*
> - * magic number and level verification
> - *
> - * During growfs operations, we can't verify the exact level or owner as
> - * the perag is not fully initialised and hence not attached to the
> - * buffer. In this case, check against the maximum tree depth.
> + * The perag may not be attached during grow operations or fully
> + * initialized from the AGF during log recovery. Therefore we can only
> + * check against maximum tree depth from those contexts.
> *
> - * Similarly, during log recovery we will have a perag structure
> - * attached, but the agf information will not yet have been initialised
> - * from the on disk AGF. Again, we can only check against maximum limits
> - * in this case.
> + * Otherwise check against the per-tree limit. Peek at one of the
> + * verifier magic values to determine the type of tree we're verifying
> + * against.
> */
> level = be16_to_cpu(block->bb_level);
> - switch (block->bb_magic) {
> - case cpu_to_be32(XFS_ABTB_CRC_MAGIC):
> - fa = xfs_btree_sblock_v5hdr_verify(bp);
> - if (fa)
> - return fa;
> - /* fall through */
> - case cpu_to_be32(XFS_ABTB_MAGIC):
> - if (pag && pag->pagf_init) {
> - if (level >= pag->pagf_levels[XFS_BTNUM_BNOi])
> - return __this_address;
> - } else if (level >= mp->m_ag_maxlevels)
> + if (bp->b_ops->magic[0] == cpu_to_be32(XFS_ABTC_MAGIC))
> + btnum = XFS_BTNUM_CNTi;
> + if (pag && pag->pagf_init) {
> + if (level >= pag->pagf_levels[btnum])
> return __this_address;
> - break;
> - case cpu_to_be32(XFS_ABTC_CRC_MAGIC):
> - fa = xfs_btree_sblock_v5hdr_verify(bp);
> - if (fa)
> - return fa;
> - /* fall through */
> - case cpu_to_be32(XFS_ABTC_MAGIC):
> - if (pag && pag->pagf_init) {
> - if (level >= pag->pagf_levels[XFS_BTNUM_CNTi])
> - return __this_address;
> - } else if (level >= mp->m_ag_maxlevels)
> - return __this_address;
> - break;
> - default:
> + } else if (level >= mp->m_ag_maxlevels)
> return __this_address;
> - }
>
> return xfs_btree_sblock_verify(bp, mp->m_alloc_mxr[level != 0]);
> }
> @@ -379,6 +365,8 @@ xfs_allocbt_write_verify(
>
> const struct xfs_buf_ops xfs_bnobt_buf_ops = {
> .name = "xfs_bnobt",
> + .magic = { cpu_to_be32(XFS_ABTB_MAGIC),
> + cpu_to_be32(XFS_ABTB_CRC_MAGIC) },
> .verify_read = xfs_allocbt_read_verify,
> .verify_write = xfs_allocbt_write_verify,
> .verify_struct = xfs_allocbt_verify,
> @@ -386,6 +374,8 @@ const struct xfs_buf_ops xfs_bnobt_buf_ops = {
>
> const struct xfs_buf_ops xfs_cntbt_buf_ops = {
> .name = "xfs_cntbt",
> + .magic = { cpu_to_be32(XFS_ABTC_MAGIC),
> + cpu_to_be32(XFS_ABTC_CRC_MAGIC) },
> .verify_read = xfs_allocbt_read_verify,
> .verify_write = xfs_allocbt_write_verify,
> .verify_struct = xfs_allocbt_verify,
> --
> 2.17.2
>
