From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/4] xfs: refactor data device extent validation
Date: Mon, 7 Dec 2020 12:46:07 -0500 [thread overview]
Message-ID: <20201207174607.GB1598552@bfoster> (raw)
In-Reply-To: <160729625702.1608297.4480089333393399990.stgit@magnolia>
On Sun, Dec 06, 2020 at 03:10:57PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor all the open-coded validation of non-static data device extents
> into a single helper.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_bmap.c | 8 ++------
> fs/xfs/libxfs/xfs_types.c | 23 +++++++++++++++++++++++
> fs/xfs/libxfs/xfs_types.h | 2 ++
> fs/xfs/scrub/bmap.c | 5 +----
> fs/xfs/xfs_bmap_item.c | 11 +----------
> fs/xfs/xfs_extfree_item.c | 11 +----------
> fs/xfs/xfs_refcount_item.c | 11 +----------
> fs/xfs/xfs_rmap_item.c | 11 +----------
> 8 files changed, 32 insertions(+), 50 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index de9c27ef68d8..7f1b6ad570a9 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6242,12 +6242,8 @@ xfs_bmap_validate_extent(
> if (!xfs_verify_rtbno(mp, endfsb))
> return __this_address;
> } else {
> - if (!xfs_verify_fsbno(mp, irec->br_startblock))
> - return __this_address;
> - if (!xfs_verify_fsbno(mp, endfsb))
> - return __this_address;
> - if (XFS_FSB_TO_AGNO(mp, irec->br_startblock) !=
> - XFS_FSB_TO_AGNO(mp, endfsb))
> + if (!xfs_verify_fsbext(mp, irec->br_startblock,
> + irec->br_blockcount))
> return __this_address;
> }
> if (irec->br_state != XFS_EXT_NORM && whichfork != XFS_DATA_FORK)
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> index 4f595546a639..b74866dbea94 100644
> --- a/fs/xfs/libxfs/xfs_types.c
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -61,6 +61,29 @@ xfs_verify_fsbno(
> return xfs_verify_agbno(mp, agno, XFS_FSB_TO_AGBNO(mp, fsbno));
> }
>
> +/*
> + * Verify that a data device extent is fully contained inside the filesystem,
> + * does not cross an AG boundary, and does not point at static metadata.
> + */
> +bool
> +xfs_verify_fsbext(
> + struct xfs_mount *mp,
> + xfs_fsblock_t fsbno,
> + xfs_fsblock_t len)
> +{
> + if (fsbno + len <= fsbno)
> + return false;
> +
> + if (!xfs_verify_fsbno(mp, fsbno))
> + return false;
> +
> + if (!xfs_verify_fsbno(mp, fsbno + len - 1))
> + return false;
> +
> + return XFS_FSB_TO_AGNO(mp, fsbno) ==
> + XFS_FSB_TO_AGNO(mp, fsbno + len - 1);
> +}
> +
> /* Calculate the first and last possible inode number in an AG. */
> void
> xfs_agino_range(
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 397d94775440..7feaaac25b3d 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -184,6 +184,8 @@ xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
> bool xfs_verify_agbno(struct xfs_mount *mp, xfs_agnumber_t agno,
> xfs_agblock_t agbno);
> bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
> +bool xfs_verify_fsbext(struct xfs_mount *mp, xfs_fsblock_t fsbno,
> + xfs_fsblock_t len);
>
> void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
> xfs_agino_t *first, xfs_agino_t *last);
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index fed56d213a3f..3e2ba7875059 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -359,10 +359,7 @@ xchk_bmap_iextent(
> xchk_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, end) ||
> - XFS_FSB_TO_AGNO(mp, irec->br_startblock) !=
> - XFS_FSB_TO_AGNO(mp, end)))
> + !xfs_verify_fsbext(mp, irec->br_startblock, irec->br_blockcount))
> xchk_fblock_set_corrupt(info->sc, info->whichfork,
> irec->br_startoff);
>
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 8d3ed07800f6..5c9706760e68 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -452,16 +452,7 @@ xfs_bui_validate(
> if (bmap->me_startoff + bmap->me_len <= bmap->me_startoff)
> return false;
>
> - if (bmap->me_startblock + bmap->me_len <= bmap->me_startblock)
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, bmap->me_startblock))
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, bmap->me_startblock + bmap->me_len - 1))
> - return false;
> -
> - return true;
> + return xfs_verify_fsbext(mp, bmap->me_startblock, bmap->me_len);
> }
>
> /*
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index bfdfbd192a38..93223ebb3372 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -584,16 +584,7 @@ xfs_efi_validate_ext(
> struct xfs_mount *mp,
> struct xfs_extent *extp)
> {
> - if (extp->ext_start + extp->ext_len <= extp->ext_start)
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, extp->ext_start))
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, extp->ext_start + extp->ext_len - 1))
> - return false;
> -
> - return true;
> + return xfs_verify_fsbext(mp, extp->ext_start, extp->ext_len);
> }
>
> /*
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 937d482c9be4..07ebccbbf4df 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -439,16 +439,7 @@ xfs_cui_validate_phys(
> return false;
> }
>
> - if (refc->pe_startblock + refc->pe_len <= refc->pe_startblock)
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, refc->pe_startblock))
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, refc->pe_startblock + refc->pe_len - 1))
> - return false;
> -
> - return true;
> + return xfs_verify_fsbext(mp, refc->pe_startblock, refc->pe_len);
> }
>
> /*
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 9b84017184d9..4fa875237422 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -493,16 +493,7 @@ xfs_rui_validate_map(
> if (rmap->me_startoff + rmap->me_len <= rmap->me_startoff)
> return false;
>
> - if (rmap->me_startblock + rmap->me_len <= rmap->me_startblock)
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, rmap->me_startblock))
> - return false;
> -
> - if (!xfs_verify_fsbno(mp, rmap->me_startblock + rmap->me_len - 1))
> - return false;
> -
> - return true;
> + return xfs_verify_fsbext(mp, rmap->me_startblock, rmap->me_len);
> }
>
> /*
>
next prev parent reply other threads:[~2020-12-07 17:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-06 23:10 [PATCH 0/4] xfs: refactor extent validation for 5.11 Darrick J. Wong
2020-12-06 23:10 ` [PATCH 1/4] xfs: refactor data device extent validation Darrick J. Wong
2020-12-06 23:49 ` Dave Chinner
2020-12-07 14:15 ` Christoph Hellwig
2020-12-07 17:46 ` Brian Foster [this message]
2020-12-06 23:11 ` [PATCH 2/4] xfs: refactor realtime volume " Darrick J. Wong
2020-12-06 23:51 ` Dave Chinner
2020-12-07 14:16 ` Christoph Hellwig
2020-12-07 17:46 ` Brian Foster
2020-12-06 23:11 ` [PATCH 3/4] xfs: refactor file range validation Darrick J. Wong
2020-12-06 23:56 ` Dave Chinner
2020-12-07 14:17 ` Christoph Hellwig
2020-12-07 17:46 ` Brian Foster
2020-12-06 23:11 ` [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_* Darrick J. Wong
2020-12-06 23:56 ` Dave Chinner
2020-12-07 14:17 ` Christoph Hellwig
2020-12-07 17:46 ` Brian Foster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201207174607.GB1598552@bfoster \
--to=bfoster@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.