Re: [PATCH] xfs: add a few more verifier tests

Christoph Hellwig
2014-08-19T18:15:53Z

> Anyway - bounds checking when we read from disk is a good thing!

Absolutelt!

Looks good modulo a few nitpicks below.

Reviewed-by: Christoph Hellwig <hch@lst.de>

> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 4bffffe..a4a9e0e 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2209,6 +2209,10 @@ xfs_agf_verify(
> be32_to_cpu(agf->agf_flcount) <= XFS_AGFL_SIZE(mp)))
> return false;
>
> + if (!(be32_to_cpu(agf->agf_levels[XFS_BTNUM_BNO]) <= XFS_BTREE_MAXLEVELS &&
> + be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNT]) <= XFS_BTREE_MAXLEVELS))
> + return false;

Maybe it's just me, but negated numeric comparisms always confuse the
hell out of me, why not simply:

if (be32_to_cpu(agf->agf_levels[XFS_BTNUM_BNO]) > XFS_BTREE_MAXLEVELS)
return false;
if (be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNT]) > XFS_BTREE_MAXLEVELS)
return false;

> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2051,6 +2051,8 @@ xfs_agi_verify(
> if (!XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum)))
> return false;
>
> + if (!(be32_to_cpu(agi->agi_level) <= XFS_BTREE_MAXLEVELS))
> + return false;

Same here.