| Message ID | 20210621223436.GF3619569@locust (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: fix endianness issue in xfs_ag_shrink_space | expand |
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,
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
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
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