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

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 22, 2021 at 09:17:19AM +1000, Dave Chinner wrote:
> 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...

woo!

--D

> Cheers,
> -- 
> Dave Chinner
> david@fromorbit.com