Re: [PATCH] xfs: fix endianness issue in xfs_ag_shrink_space

[Dave Chinner <david@fromorbit.com>]

On Mon, Jun 21, 2021 at 03:34:36PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> The AGI buffer is in big-endian format, so we must convert the
> endianness to CPU format to do any comparisons.
> 
> Fixes: 46141dc891f7 ("xfs: introduce xfs_ag_shrink_space()")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/libxfs/xfs_ag.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index c68a36688474..afff2ab7e9f1 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -510,6 +510,7 @@ xfs_ag_shrink_space(
>  	struct xfs_buf		*agibp, *agfbp;
>  	struct xfs_agi		*agi;
>  	struct xfs_agf		*agf;
> +	xfs_agblock_t		aglen;
>  	int			error, err2;
>  
>  	ASSERT(agno == mp->m_sb.sb_agcount - 1);
> @@ -524,14 +525,14 @@ xfs_ag_shrink_space(
>  		return error;
>  
>  	agf = agfbp->b_addr;
> +	aglen = be32_to_cpu(agi->agi_length);
>  	/* some extra paranoid checks before we shrink the ag */
>  	if (XFS_IS_CORRUPT(mp, agf->agf_length != agi->agi_length))
>  		return -EFSCORRUPTED;
> -	if (delta >= agi->agi_length)
> +	if (delta >= aglen)
>  		return -EINVAL;
>  
> -	args.fsbno = XFS_AGB_TO_FSB(mp, agno,
> -				    be32_to_cpu(agi->agi_length) - delta);
> +	args.fsbno = XFS_AGB_TO_FSB(mp, agno, aglen - delta);

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

FWIW, my plan for this stuff is to move the perag geometry stuff
into the xfs_perag. That gets rid of all this "need the on disk
buffer to get AG size" stuff. It also avoids having to calculate
valid ranges of types on every verify call (expensive) because, at
most per-ag type verifier call sites, we already have the perag on
hand...

Cheers,
-- 
Dave Chinner
david@fromorbit.com