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

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

> 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@xxxxxx>

> 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.