[PATCH] xfs: fix endianness issue in xfs_ag_shrink_space

[PATCH] xfs: fix endianness issue in xfs_ag_shrink_space

[Darrick J. Wong]

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);
 
 	/*
 	 * Disable perag reservations so it doesn't cause the allocation request

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

[Dave Chinner]

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

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

[Gao Xiang]

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

LGTM,
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

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

[Darrick J. Wong]

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