All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] xfs: refactor extent validation for 5.11
@ 2020-12-06 23:10 Darrick J. Wong
  2020-12-06 23:10 ` [PATCH 1/4] xfs: refactor data device extent validation Darrick J. Wong
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-12-06 23:10 UTC (permalink / raw)
  To: darrick.wong; +Cc: Dave Chinner, linux-xfs

Hi all,

While reviewing the "strengthen log intent validation" series, Brian
Foster suggested that we should refactor all the validation code that
checks physical extents into a common helper, so I've done that for both
data and rt volume mappings.  I also did the same for file ranges, since
those were kind of a mess of open-coded stuff.  Dave Chinner also asked
to rename xfs_fc -> xfs_fs so that's the last patch.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=refactorings-5.11
---
 fs/xfs/libxfs/xfs_bmap.c   |   23 ++++------------
 fs/xfs/libxfs/xfs_types.c  |   64 ++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_types.h  |    7 +++++
 fs/xfs/scrub/bmap.c        |   17 ++++--------
 fs/xfs/scrub/rtbitmap.c    |    4 +--
 fs/xfs/xfs_bmap_item.c     |   13 +--------
 fs/xfs/xfs_extfree_item.c  |   11 +-------
 fs/xfs/xfs_refcount_item.c |   11 +-------
 fs/xfs/xfs_rmap_item.c     |   13 +--------
 fs/xfs/xfs_super.c         |   26 +++++++++---------
 10 files changed, 103 insertions(+), 86 deletions(-)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4] xfs: refactor data device extent validation
  2020-12-06 23:10 [PATCH 0/4] xfs: refactor extent validation for 5.11 Darrick J. Wong
@ 2020-12-06 23:10 ` Darrick J. Wong
  2020-12-06 23:49   ` Dave Chinner
                     ` (2 more replies)
  2020-12-06 23:11 ` [PATCH 2/4] xfs: refactor realtime volume " Darrick J. Wong
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-12-06 23:10 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

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>
---
 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);
 }
 
 /*


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] xfs: refactor realtime volume extent validation
  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:11 ` Darrick J. Wong
  2020-12-06 23:51   ` Dave Chinner
                     ` (2 more replies)
  2020-12-06 23:11 ` [PATCH 3/4] xfs: refactor file range validation Darrick J. Wong
  2020-12-06 23:11 ` [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_* Darrick J. Wong
  3 siblings, 3 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-12-06 23:11 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

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


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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4] xfs: refactor file range validation
  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:11 ` [PATCH 2/4] xfs: refactor realtime volume " Darrick J. Wong
@ 2020-12-06 23:11 ` Darrick J. Wong
  2020-12-06 23:56   ` Dave Chinner
                     ` (2 more replies)
  2020-12-06 23:11 ` [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_* Darrick J. Wong
  3 siblings, 3 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-12-06 23:11 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor all the open-coded validation of file block ranges into a
single helper, and teach the bmap scrubber to check the ranges.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c  |    2 +-
 fs/xfs/libxfs/xfs_types.c |   25 +++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_types.h |    3 +++
 fs/xfs/scrub/bmap.c       |    4 ++++
 fs/xfs/xfs_bmap_item.c    |    2 +-
 fs/xfs/xfs_rmap_item.c    |    2 +-
 6 files changed, 35 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7bcf498ef6b2..dcf56bcafb8f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -6227,7 +6227,7 @@ xfs_bmap_validate_extent(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 
-	if (irec->br_startoff + irec->br_blockcount <= irec->br_startoff)
+	if (!xfs_verify_fileext(mp, irec->br_startoff, irec->br_blockcount))
 		return __this_address;
 
 	if (XFS_IS_REALTIME_INODE(ip) && whichfork == XFS_DATA_FORK) {
diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
index 7b310eb296b7..b254fbeaaa50 100644
--- a/fs/xfs/libxfs/xfs_types.c
+++ b/fs/xfs/libxfs/xfs_types.c
@@ -258,3 +258,28 @@ xfs_verify_dablk(
 
 	return dabno <= max_dablk;
 }
+
+/* Check that a file block offset does not exceed the maximum. */
+bool
+xfs_verify_fileoff(
+	struct xfs_mount	*mp,
+	xfs_fileoff_t		off)
+{
+	return off <= XFS_MAX_FILEOFF;
+}
+
+/* Check that a range of file block offsets do not exceed the maximum. */
+bool
+xfs_verify_fileext(
+	struct xfs_mount	*mp,
+	xfs_fileoff_t		off,
+	xfs_fileoff_t		len)
+{
+	if (off + len <= off)
+		return false;
+
+	if (!xfs_verify_fileoff(mp, off))
+		return false;
+
+	return xfs_verify_fileoff(mp, off + len - 1);
+}
diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index 18e83ce46568..064bd6e8c922 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -203,5 +203,8 @@ 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,
 		unsigned long long *max);
+bool xfs_verify_fileoff(struct xfs_mount *mp, xfs_fileoff_t off);
+bool xfs_verify_fileext(struct xfs_mount *mp, xfs_fileoff_t off,
+		xfs_fileoff_t len);
 
 #endif	/* __XFS_TYPES_H__ */
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index cce8ac7d3973..bce4421acdb9 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -329,6 +329,10 @@ xchk_bmap_iextent(
 		xchk_fblock_set_corrupt(info->sc, info->whichfork,
 				irec->br_startoff);
 
+	if (!xfs_verify_fileext(mp, irec->br_startoff, irec->br_blockcount))
+		xchk_fblock_set_corrupt(info->sc, info->whichfork,
+				irec->br_startoff);
+
 	xchk_bmap_dirattr_extent(ip, info, irec);
 
 	/* There should never be a "hole" extent in either extent list. */
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 5c9706760e68..9a2e54b7ccb9 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -449,7 +449,7 @@ xfs_bui_validate(
 	if (!xfs_verify_ino(mp, bmap->me_owner))
 		return false;
 
-	if (bmap->me_startoff + bmap->me_len <= bmap->me_startoff)
+	if (!xfs_verify_fileext(mp, bmap->me_startoff, bmap->me_len))
 		return false;
 
 	return xfs_verify_fsbext(mp, bmap->me_startblock, bmap->me_len);
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 4fa875237422..49cebd68b672 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -490,7 +490,7 @@ xfs_rui_validate_map(
 	    !xfs_verify_ino(mp, rmap->me_owner))
 		return false;
 
-	if (rmap->me_startoff + rmap->me_len <= rmap->me_startoff)
+	if (!xfs_verify_fileext(mp, rmap->me_startoff, rmap->me_len))
 		return false;
 
 	return xfs_verify_fsbext(mp, rmap->me_startblock, rmap->me_len);


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_*
  2020-12-06 23:10 [PATCH 0/4] xfs: refactor extent validation for 5.11 Darrick J. Wong
                   ` (2 preceding siblings ...)
  2020-12-06 23:11 ` [PATCH 3/4] xfs: refactor file range validation Darrick J. Wong
@ 2020-12-06 23:11 ` Darrick J. Wong
  2020-12-06 23:56   ` Dave Chinner
                     ` (2 more replies)
  3 siblings, 3 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-12-06 23:11 UTC (permalink / raw)
  To: darrick.wong; +Cc: Dave Chinner, linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Get rid of this one-off namespace since we're done converting things to
fscontext now.

Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)


diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 36002f460d7c..12d6850aedc6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1159,7 +1159,7 @@ suffix_kstrtoint(
  * NOTE: mp->m_super is NULL here!
  */
 static int
-xfs_fc_parse_param(
+xfs_fs_parse_param(
 	struct fs_context	*fc,
 	struct fs_parameter	*param)
 {
@@ -1317,7 +1317,7 @@ xfs_fc_parse_param(
 }
 
 static int
-xfs_fc_validate_params(
+xfs_fs_validate_params(
 	struct xfs_mount	*mp)
 {
 	/*
@@ -1386,7 +1386,7 @@ xfs_fc_validate_params(
 }
 
 static int
-xfs_fc_fill_super(
+xfs_fs_fill_super(
 	struct super_block	*sb,
 	struct fs_context	*fc)
 {
@@ -1396,7 +1396,7 @@ xfs_fc_fill_super(
 
 	mp->m_super = sb;
 
-	error = xfs_fc_validate_params(mp);
+	error = xfs_fs_validate_params(mp);
 	if (error)
 		goto out_free_names;
 
@@ -1660,10 +1660,10 @@ xfs_fc_fill_super(
 }
 
 static int
-xfs_fc_get_tree(
+xfs_fs_get_tree(
 	struct fs_context	*fc)
 {
-	return get_tree_bdev(fc, xfs_fc_fill_super);
+	return get_tree_bdev(fc, xfs_fs_fill_super);
 }
 
 static int
@@ -1782,7 +1782,7 @@ xfs_remount_ro(
  * silently ignore all options that we can't actually change.
  */
 static int
-xfs_fc_reconfigure(
+xfs_fs_reconfigure(
 	struct fs_context *fc)
 {
 	struct xfs_mount	*mp = XFS_M(fc->root->d_sb);
@@ -1795,7 +1795,7 @@ xfs_fc_reconfigure(
 	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
 		fc->sb_flags |= SB_I_VERSION;
 
-	error = xfs_fc_validate_params(new_mp);
+	error = xfs_fs_validate_params(new_mp);
 	if (error)
 		return error;
 
@@ -1832,7 +1832,7 @@ xfs_fc_reconfigure(
 	return 0;
 }
 
-static void xfs_fc_free(
+static void xfs_fs_free(
 	struct fs_context	*fc)
 {
 	struct xfs_mount	*mp = fc->s_fs_info;
@@ -1848,10 +1848,10 @@ static void xfs_fc_free(
 }
 
 static const struct fs_context_operations xfs_context_ops = {
-	.parse_param = xfs_fc_parse_param,
-	.get_tree    = xfs_fc_get_tree,
-	.reconfigure = xfs_fc_reconfigure,
-	.free        = xfs_fc_free,
+	.parse_param = xfs_fs_parse_param,
+	.get_tree    = xfs_fs_get_tree,
+	.reconfigure = xfs_fs_reconfigure,
+	.free        = xfs_fs_free,
 };
 
 static int xfs_init_fs_context(


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] xfs: refactor data device extent validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2020-12-06 23:49 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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>

Nice cleanup.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] xfs: refactor realtime volume extent validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2020-12-06 23:51 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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>
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] xfs: refactor file range validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2020-12-06 23:56 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Sun, Dec 06, 2020 at 03:11:09PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor all the open-coded validation of file block ranges into a
> single helper, and teach the bmap scrubber to check the ranges.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c  |    2 +-
>  fs/xfs/libxfs/xfs_types.c |   25 +++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_types.h |    3 +++
>  fs/xfs/scrub/bmap.c       |    4 ++++
>  fs/xfs/xfs_bmap_item.c    |    2 +-
>  fs/xfs/xfs_rmap_item.c    |    2 +-
>  6 files changed, 35 insertions(+), 3 deletions(-)

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_*
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2020-12-06 23:56 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Sun, Dec 06, 2020 at 03:11:15PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Get rid of this one-off namespace since we're done converting things to
> fscontext now.
> 
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Yay!

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] xfs: refactor data device extent validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-12-07 14:15 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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.

looks good, but I think a little convenience wrapper that takes a
xfs_bmbt_irec * would also be really helpful.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] xfs: refactor realtime volume extent validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-12-07 14:16 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

Same comment as for patch 1.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] xfs: refactor file range validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-12-07 14:17 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

> +/* Check that a file block offset does not exceed the maximum. */
> +bool
> +xfs_verify_fileoff(
> +	struct xfs_mount	*mp,
> +	xfs_fileoff_t		off)
> +{
> +	return off <= XFS_MAX_FILEOFF;
> +}

I think an inline function would make sense here..

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_*
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-12-07 14:17 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Dave Chinner, linux-xfs

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] xfs: refactor data device extent validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Brian Foster @ 2020-12-07 17:46 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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);
>  }
>  
>  /*
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] xfs: refactor realtime volume extent validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Brian Foster @ 2020-12-07 17:46 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] xfs: refactor file range validation
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Brian Foster @ 2020-12-07 17:46 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Sun, Dec 06, 2020 at 03:11:09PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor all the open-coded validation of file block ranges into a
> single helper, and teach the bmap scrubber to check the ranges.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_bmap.c  |    2 +-
>  fs/xfs/libxfs/xfs_types.c |   25 +++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_types.h |    3 +++
>  fs/xfs/scrub/bmap.c       |    4 ++++
>  fs/xfs/xfs_bmap_item.c    |    2 +-
>  fs/xfs/xfs_rmap_item.c    |    2 +-
>  6 files changed, 35 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 7bcf498ef6b2..dcf56bcafb8f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -6227,7 +6227,7 @@ xfs_bmap_validate_extent(
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  
> -	if (irec->br_startoff + irec->br_blockcount <= irec->br_startoff)
> +	if (!xfs_verify_fileext(mp, irec->br_startoff, irec->br_blockcount))
>  		return __this_address;
>  
>  	if (XFS_IS_REALTIME_INODE(ip) && whichfork == XFS_DATA_FORK) {
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> index 7b310eb296b7..b254fbeaaa50 100644
> --- a/fs/xfs/libxfs/xfs_types.c
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -258,3 +258,28 @@ xfs_verify_dablk(
>  
>  	return dabno <= max_dablk;
>  }
> +
> +/* Check that a file block offset does not exceed the maximum. */
> +bool
> +xfs_verify_fileoff(
> +	struct xfs_mount	*mp,
> +	xfs_fileoff_t		off)
> +{
> +	return off <= XFS_MAX_FILEOFF;
> +}
> +
> +/* Check that a range of file block offsets do not exceed the maximum. */
> +bool
> +xfs_verify_fileext(
> +	struct xfs_mount	*mp,
> +	xfs_fileoff_t		off,
> +	xfs_fileoff_t		len)
> +{
> +	if (off + len <= off)
> +		return false;
> +
> +	if (!xfs_verify_fileoff(mp, off))
> +		return false;
> +
> +	return xfs_verify_fileoff(mp, off + len - 1);
> +}
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 18e83ce46568..064bd6e8c922 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -203,5 +203,8 @@ 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,
>  		unsigned long long *max);
> +bool xfs_verify_fileoff(struct xfs_mount *mp, xfs_fileoff_t off);
> +bool xfs_verify_fileext(struct xfs_mount *mp, xfs_fileoff_t off,
> +		xfs_fileoff_t len);
>  
>  #endif	/* __XFS_TYPES_H__ */
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index cce8ac7d3973..bce4421acdb9 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -329,6 +329,10 @@ xchk_bmap_iextent(
>  		xchk_fblock_set_corrupt(info->sc, info->whichfork,
>  				irec->br_startoff);
>  
> +	if (!xfs_verify_fileext(mp, irec->br_startoff, irec->br_blockcount))
> +		xchk_fblock_set_corrupt(info->sc, info->whichfork,
> +				irec->br_startoff);
> +
>  	xchk_bmap_dirattr_extent(ip, info, irec);
>  
>  	/* There should never be a "hole" extent in either extent list. */
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 5c9706760e68..9a2e54b7ccb9 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -449,7 +449,7 @@ xfs_bui_validate(
>  	if (!xfs_verify_ino(mp, bmap->me_owner))
>  		return false;
>  
> -	if (bmap->me_startoff + bmap->me_len <= bmap->me_startoff)
> +	if (!xfs_verify_fileext(mp, bmap->me_startoff, bmap->me_len))
>  		return false;
>  
>  	return xfs_verify_fsbext(mp, bmap->me_startblock, bmap->me_len);
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 4fa875237422..49cebd68b672 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -490,7 +490,7 @@ xfs_rui_validate_map(
>  	    !xfs_verify_ino(mp, rmap->me_owner))
>  		return false;
>  
> -	if (rmap->me_startoff + rmap->me_len <= rmap->me_startoff)
> +	if (!xfs_verify_fileext(mp, rmap->me_startoff, rmap->me_len))
>  		return false;
>  
>  	return xfs_verify_fsbext(mp, rmap->me_startblock, rmap->me_len);
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] xfs: rename xfs_fc_* back to xfs_fs_*
  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
  2 siblings, 0 replies; 17+ messages in thread
From: Brian Foster @ 2020-12-07 17:46 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Dave Chinner, linux-xfs

On Sun, Dec 06, 2020 at 03:11:15PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Get rid of this one-off namespace since we're done converting things to
> fscontext now.
> 
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_super.c |   26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 36002f460d7c..12d6850aedc6 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1159,7 +1159,7 @@ suffix_kstrtoint(
>   * NOTE: mp->m_super is NULL here!
>   */
>  static int
> -xfs_fc_parse_param(
> +xfs_fs_parse_param(
>  	struct fs_context	*fc,
>  	struct fs_parameter	*param)
>  {
> @@ -1317,7 +1317,7 @@ xfs_fc_parse_param(
>  }
>  
>  static int
> -xfs_fc_validate_params(
> +xfs_fs_validate_params(
>  	struct xfs_mount	*mp)
>  {
>  	/*
> @@ -1386,7 +1386,7 @@ xfs_fc_validate_params(
>  }
>  
>  static int
> -xfs_fc_fill_super(
> +xfs_fs_fill_super(
>  	struct super_block	*sb,
>  	struct fs_context	*fc)
>  {
> @@ -1396,7 +1396,7 @@ xfs_fc_fill_super(
>  
>  	mp->m_super = sb;
>  
> -	error = xfs_fc_validate_params(mp);
> +	error = xfs_fs_validate_params(mp);
>  	if (error)
>  		goto out_free_names;
>  
> @@ -1660,10 +1660,10 @@ xfs_fc_fill_super(
>  }
>  
>  static int
> -xfs_fc_get_tree(
> +xfs_fs_get_tree(
>  	struct fs_context	*fc)
>  {
> -	return get_tree_bdev(fc, xfs_fc_fill_super);
> +	return get_tree_bdev(fc, xfs_fs_fill_super);
>  }
>  
>  static int
> @@ -1782,7 +1782,7 @@ xfs_remount_ro(
>   * silently ignore all options that we can't actually change.
>   */
>  static int
> -xfs_fc_reconfigure(
> +xfs_fs_reconfigure(
>  	struct fs_context *fc)
>  {
>  	struct xfs_mount	*mp = XFS_M(fc->root->d_sb);
> @@ -1795,7 +1795,7 @@ xfs_fc_reconfigure(
>  	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
>  		fc->sb_flags |= SB_I_VERSION;
>  
> -	error = xfs_fc_validate_params(new_mp);
> +	error = xfs_fs_validate_params(new_mp);
>  	if (error)
>  		return error;
>  
> @@ -1832,7 +1832,7 @@ xfs_fc_reconfigure(
>  	return 0;
>  }
>  
> -static void xfs_fc_free(
> +static void xfs_fs_free(
>  	struct fs_context	*fc)
>  {
>  	struct xfs_mount	*mp = fc->s_fs_info;
> @@ -1848,10 +1848,10 @@ static void xfs_fc_free(
>  }
>  
>  static const struct fs_context_operations xfs_context_ops = {
> -	.parse_param = xfs_fc_parse_param,
> -	.get_tree    = xfs_fc_get_tree,
> -	.reconfigure = xfs_fc_reconfigure,
> -	.free        = xfs_fc_free,
> +	.parse_param = xfs_fs_parse_param,
> +	.get_tree    = xfs_fs_get_tree,
> +	.reconfigure = xfs_fs_reconfigure,
> +	.free        = xfs_fs_free,
>  };
>  
>  static int xfs_init_fs_context(
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-12-07 17:48 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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.