diff mbox series

[2/4] xfs: refactor realtime volume extent validation

Message ID 160729626316.1608297.11622795343009336589.stgit@magnolia (mailing list archive)
State Accepted, archived
Headers show
Series xfs: refactor extent validation for 5.11 | expand

Commit Message

Darrick J. Wong Dec. 6, 2020, 11:11 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor all the open-coded validation of realtime device extents into a
single helper.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c  |   13 +++----------
 fs/xfs/libxfs/xfs_types.c |   16 ++++++++++++++++
 fs/xfs/libxfs/xfs_types.h |    2 ++
 fs/xfs/scrub/bmap.c       |    8 +-------
 fs/xfs/scrub/rtbitmap.c   |    4 +---
 5 files changed, 23 insertions(+), 20 deletions(-)

Comments

Dave Chinner Dec. 6, 2020, 11:51 p.m. UTC | #1 
On Sun, Dec 06, 2020 at 03:11:03PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor all the open-coded validation of realtime device extents into a
> single helper.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
Christoph Hellwig Dec. 7, 2020, 2:16 p.m. UTC | #2 
Same comment as for patch 1.
Brian Foster Dec. 7, 2020, 5:46 p.m. UTC | #3 
On Sun, Dec 06, 2020 at 03:11:03PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor all the open-coded validation of realtime 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  |   13 +++----------
>  fs/xfs/libxfs/xfs_types.c |   16 ++++++++++++++++
>  fs/xfs/libxfs/xfs_types.h |    2 ++
>  fs/xfs/scrub/bmap.c       |    8 +-------
>  fs/xfs/scrub/rtbitmap.c   |    4 +---
>  5 files changed, 23 insertions(+), 20 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 7f1b6ad570a9..7bcf498ef6b2 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6226,20 +6226,13 @@ xfs_bmap_validate_extent(
>  	struct xfs_bmbt_irec	*irec)
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
> -	xfs_fsblock_t		endfsb;
> -	bool			isrt;
>  
> -	if (irec->br_startblock + irec->br_blockcount <= irec->br_startblock)
> -		return __this_address;
>  	if (irec->br_startoff + irec->br_blockcount <= irec->br_startoff)
>  		return __this_address;
>  
> -	isrt = XFS_IS_REALTIME_INODE(ip);
> -	endfsb = irec->br_startblock + irec->br_blockcount - 1;
> -	if (isrt && whichfork == XFS_DATA_FORK) {
> -		if (!xfs_verify_rtbno(mp, irec->br_startblock))
> -			return __this_address;
> -		if (!xfs_verify_rtbno(mp, endfsb))
> +	if (XFS_IS_REALTIME_INODE(ip) && whichfork == XFS_DATA_FORK) {
> +		if (!xfs_verify_rtext(mp, irec->br_startblock,
> +					  irec->br_blockcount))
>  			return __this_address;
>  	} else {
>  		if (!xfs_verify_fsbext(mp, irec->br_startblock,
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> index b74866dbea94..7b310eb296b7 100644
> --- a/fs/xfs/libxfs/xfs_types.c
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -198,6 +198,22 @@ xfs_verify_rtbno(
>  	return rtbno < mp->m_sb.sb_rblocks;
>  }
>  
> +/* Verify that a realtime device extent is fully contained inside the volume. */
> +bool
> +xfs_verify_rtext(
> +	struct xfs_mount	*mp,
> +	xfs_rtblock_t		rtbno,
> +	xfs_rtblock_t		len)
> +{
> +	if (rtbno + len <= rtbno)
> +		return false;
> +
> +	if (!xfs_verify_rtbno(mp, rtbno))
> +		return false;
> +
> +	return xfs_verify_rtbno(mp, rtbno + len - 1);
> +}
> +
>  /* Calculate the range of valid icount values. */
>  void
>  xfs_icount_range(
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 7feaaac25b3d..18e83ce46568 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -197,6 +197,8 @@ bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
>  bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
>  bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
>  bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
> +bool xfs_verify_rtext(struct xfs_mount *mp, xfs_rtblock_t rtbno,
> +		xfs_rtblock_t len);
>  bool xfs_verify_icount(struct xfs_mount *mp, unsigned long long icount);
>  bool xfs_verify_dablk(struct xfs_mount *mp, xfs_fileoff_t off);
>  void xfs_icount_range(struct xfs_mount *mp, unsigned long long *min,
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index 3e2ba7875059..cce8ac7d3973 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -319,7 +319,6 @@ xchk_bmap_iextent(
>  	struct xfs_bmbt_irec	*irec)
>  {
>  	struct xfs_mount	*mp = info->sc->mp;
> -	xfs_filblks_t		end;
>  	int			error = 0;
>  
>  	/*
> @@ -349,13 +348,8 @@ xchk_bmap_iextent(
>  	if (irec->br_blockcount > MAXEXTLEN)
>  		xchk_fblock_set_corrupt(info->sc, info->whichfork,
>  				irec->br_startoff);
> -	if (irec->br_startblock + irec->br_blockcount <= irec->br_startblock)
> -		xchk_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, end)))
> +	    !xfs_verify_rtext(mp, irec->br_startblock, irec->br_blockcount))
>  		xchk_fblock_set_corrupt(info->sc, info->whichfork,
>  				irec->br_startoff);
>  	if (!info->is_rt &&
> diff --git a/fs/xfs/scrub/rtbitmap.c b/fs/xfs/scrub/rtbitmap.c
> index 76e4ffe0315b..d409ca592178 100644
> --- a/fs/xfs/scrub/rtbitmap.c
> +++ b/fs/xfs/scrub/rtbitmap.c
> @@ -52,9 +52,7 @@ xchk_rtbitmap_rec(
>  	startblock = rec->ar_startext * tp->t_mountp->m_sb.sb_rextsize;
>  	blockcount = rec->ar_extcount * tp->t_mountp->m_sb.sb_rextsize;
>  
> -	if (startblock + blockcount <= startblock ||
> -	    !xfs_verify_rtbno(sc->mp, startblock) ||
> -	    !xfs_verify_rtbno(sc->mp, startblock + blockcount - 1))
> +	if (!xfs_verify_rtext(sc->mp, startblock, blockcount))
>  		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
>  	return 0;
>  }
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7f1b6ad570a9..7bcf498ef6b2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -6226,20 +6226,13 @@  xfs_bmap_validate_extent(
 	struct xfs_bmbt_irec	*irec)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	xfs_fsblock_t		endfsb;
-	bool			isrt;
 
-	if (irec->br_startblock + irec->br_blockcount <= irec->br_startblock)
-		return __this_address;
 	if (irec->br_startoff + irec->br_blockcount <= irec->br_startoff)
 		return __this_address;
 
-	isrt = XFS_IS_REALTIME_INODE(ip);
-	endfsb = irec->br_startblock + irec->br_blockcount - 1;
-	if (isrt && whichfork == XFS_DATA_FORK) {
-		if (!xfs_verify_rtbno(mp, irec->br_startblock))
-			return __this_address;
-		if (!xfs_verify_rtbno(mp, endfsb))
+	if (XFS_IS_REALTIME_INODE(ip) && whichfork == XFS_DATA_FORK) {
+		if (!xfs_verify_rtext(mp, irec->br_startblock,
+					  irec->br_blockcount))
 			return __this_address;
 	} else {
 		if (!xfs_verify_fsbext(mp, irec->br_startblock,
diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
index b74866dbea94..7b310eb296b7 100644
--- a/fs/xfs/libxfs/xfs_types.c
+++ b/fs/xfs/libxfs/xfs_types.c
@@ -198,6 +198,22 @@  xfs_verify_rtbno(
 	return rtbno < mp->m_sb.sb_rblocks;
 }
 
+/* Verify that a realtime device extent is fully contained inside the volume. */
+bool
+xfs_verify_rtext(
+	struct xfs_mount	*mp,
+	xfs_rtblock_t		rtbno,
+	xfs_rtblock_t		len)
+{
+	if (rtbno + len <= rtbno)
+		return false;
+
+	if (!xfs_verify_rtbno(mp, rtbno))
+		return false;
+
+	return xfs_verify_rtbno(mp, rtbno + len - 1);
+}
+
 /* Calculate the range of valid icount values. */
 void
 xfs_icount_range(
diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index 7feaaac25b3d..18e83ce46568 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -197,6 +197,8 @@  bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
 bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
 bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
 bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
+bool xfs_verify_rtext(struct xfs_mount *mp, xfs_rtblock_t rtbno,
+		xfs_rtblock_t len);
 bool xfs_verify_icount(struct xfs_mount *mp, unsigned long long icount);
 bool xfs_verify_dablk(struct xfs_mount *mp, xfs_fileoff_t off);
 void xfs_icount_range(struct xfs_mount *mp, unsigned long long *min,
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 3e2ba7875059..cce8ac7d3973 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -319,7 +319,6 @@  xchk_bmap_iextent(
 	struct xfs_bmbt_irec	*irec)
 {
 	struct xfs_mount	*mp = info->sc->mp;
-	xfs_filblks_t		end;
 	int			error = 0;
 
 	/*
@@ -349,13 +348,8 @@  xchk_bmap_iextent(
 	if (irec->br_blockcount > MAXEXTLEN)
 		xchk_fblock_set_corrupt(info->sc, info->whichfork,
 				irec->br_startoff);
-	if (irec->br_startblock + irec->br_blockcount <= irec->br_startblock)
-		xchk_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, end)))
+	    !xfs_verify_rtext(mp, irec->br_startblock, irec->br_blockcount))
 		xchk_fblock_set_corrupt(info->sc, info->whichfork,
 				irec->br_startoff);
 	if (!info->is_rt &&
diff --git a/fs/xfs/scrub/rtbitmap.c b/fs/xfs/scrub/rtbitmap.c
index 76e4ffe0315b..d409ca592178 100644
--- a/fs/xfs/scrub/rtbitmap.c
+++ b/fs/xfs/scrub/rtbitmap.c
@@ -52,9 +52,7 @@  xchk_rtbitmap_rec(
 	startblock = rec->ar_startext * tp->t_mountp->m_sb.sb_rextsize;
 	blockcount = rec->ar_extcount * tp->t_mountp->m_sb.sb_rextsize;
 
-	if (startblock + blockcount <= startblock ||
-	    !xfs_verify_rtbno(sc->mp, startblock) ||
-	    !xfs_verify_rtbno(sc->mp, startblock + blockcount - 1))
+	if (!xfs_verify_rtext(sc->mp, startblock, blockcount))
 		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
 	return 0;
 }