| Message ID | 20180116233415.GH5602@magnolia (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Tue, Jan 16, 2018 at 03:34:15PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > xfs_bmbt_irec.br_blockcount is declared as xfs_filblks_t, which is an > unsigned 64-bit integer. Though the bmbt helpers will never set a value > larger than 2^21 (since the underlying on-disk extent record has a > length field that is only 21 bits wide), we should be a little defensive > about checking that a bmbt record doesn't exceed what we're expecting or > overflow into the next AG. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/scrub/bmap.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c > index 3a815db..fa64bd4 100644 > --- a/fs/xfs/scrub/bmap.c > +++ b/fs/xfs/scrub/bmap.c > @@ -348,6 +348,7 @@ xfs_scrub_bmap_extent( > { > struct xfs_mount *mp = info->sc->mp; > struct xfs_buf *bp = NULL; > + xfs_filblks_t end; > int error = 0; > > if (cur) > @@ -375,19 +376,23 @@ xfs_scrub_bmap_extent( > irec->br_startoff); > > /* Make sure the extent points to a valid place. */ > + if (irec->br_blockcount > MAXEXTLEN) > + xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, > + irec->br_startoff); > if (irec->br_startblock + irec->br_blockcount <= irec->br_startblock) > xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, > irec->br_startoff); > + end = irec->br_startblock + irec->br_blockcount - 1; > if (info->is_rt && > (!xfs_verify_rtbno(mp, irec->br_startblock) || > - !xfs_verify_rtbno(mp, irec->br_startblock + > - irec->br_blockcount - 1))) > + !xfs_verify_rtbno(mp, end))) > xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, > irec->br_startoff); > if (!info->is_rt && > (!xfs_verify_fsbno(mp, irec->br_startblock) || > - !xfs_verify_fsbno(mp, irec->br_startblock + > - irec->br_blockcount - 1))) > + !xfs_verify_fsbno(mp, end) || > + XFS_FSB_TO_AGNO(mp, irec->br_startblock) != > + XFS_FSB_TO_AGNO(mp, end))) Indent that last part of the check so it's clear the two lines are a single logic statement: XFS_FSB_TO_AGNO(mp, irec->br_startblock) != XFS_FSB_TO_AGNO(mp, end))) Otherwise it looks good. Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c index 3a815db..fa64bd4 100644 --- a/fs/xfs/scrub/bmap.c +++ b/fs/xfs/scrub/bmap.c @@ -348,6 +348,7 @@ xfs_scrub_bmap_extent( { struct xfs_mount *mp = info->sc->mp; struct xfs_buf *bp = NULL; + xfs_filblks_t end; int error = 0; if (cur) @@ -375,19 +376,23 @@ xfs_scrub_bmap_extent( irec->br_startoff); /* Make sure the extent points to a valid place. */ + if (irec->br_blockcount > MAXEXTLEN) + xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, + irec->br_startoff); if (irec->br_startblock + irec->br_blockcount <= irec->br_startblock) xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, irec->br_startoff); + end = irec->br_startblock + irec->br_blockcount - 1; if (info->is_rt && (!xfs_verify_rtbno(mp, irec->br_startblock) || - !xfs_verify_rtbno(mp, irec->br_startblock + - irec->br_blockcount - 1))) + !xfs_verify_rtbno(mp, end))) xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, irec->br_startoff); if (!info->is_rt && (!xfs_verify_fsbno(mp, irec->br_startblock) || - !xfs_verify_fsbno(mp, irec->br_startblock + - irec->br_blockcount - 1))) + !xfs_verify_fsbno(mp, end) || + XFS_FSB_TO_AGNO(mp, irec->br_startblock) != + XFS_FSB_TO_AGNO(mp, end))) xfs_scrub_fblock_set_corrupt(info->sc, info->whichfork, irec->br_startoff);