[PATCHSET 1/2] xfs: LLM-inspired bug fixes, part 22

[PATCHSET 1/2] xfs: LLM-inspired bug fixes, part 22

[Darrick J. Wong]

Hi all,

Here's a twenty-second batch of xfs fixes resulting from a LLaMma.  Mwa
mwa mwa...

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
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=llm-fixes-22
---
Commits in this patchset:
 * xfs: don't overrun incore inode when cleaning attr forks
 * xfs: fix data fork block count check in xrep_inode_blockcounts
 * xfs: avoid oversized space allocations when regenerating metadata
 * xfs: reduce new btree slack factor if free space is negative
 * xfs: don't queue rmap update twice when reaping a fork
 * xfs: only written file data blocks can be shared
 * xfs: check for fragments when full-extent sharing matches
 * xfs: don't clear COWEXTSIZE on directories on reflink filesystems
 * xfs: clear set[ug]id mode bits when zapping attr fork
 * xfs: opportunistically skip btree block checks
 * xfs: always report scrub errors in tracepoints
 * xfs: don't scan across metadata/regular directory trees for dirents
---
 fs/xfs/scrub/btree.c        |    9 ++++++++-
 fs/xfs/scrub/common.c       |    7 +++++--
 fs/xfs/scrub/dabtree.c      |    6 +++++-
 fs/xfs/scrub/dir_repair.c   |    4 +++-
 fs/xfs/scrub/inode_repair.c |   13 +++++++++++--
 fs/xfs/scrub/newbt.c        |    8 +++++---
 fs/xfs/scrub/reap.c         |    4 ++--
 fs/xfs/scrub/refcount.c     |   17 ++++++++++++++++-
 fs/xfs/scrub/rtrefcount.c   |   17 ++++++++++++++++-
 9 files changed, 71 insertions(+), 14 deletions(-)

[PATCH 01/12] xfs: don't overrun incore inode when cleaning attr forks

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that xrep_dinode_check_afork doesn't check for a
di_forkoff value that is so large that the attr fork would begin after
the end of the inode space.  This causes afork_{ptr,size} to be
nonsense.  What we really ought to do here is zap the attr fork.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: e744cef2060559 ("xfs: zap broken inode forks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode_repair.c |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 5b5116adcbf5e4..de3c915984290b 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1243,6 +1243,8 @@ xrep_dinode_check_afork(
 	if (XFS_DFORK_BOFF(dip) == 0)
 		return dip->di_aformat != XFS_DINODE_FMT_EXTENTS ||
 		       xfs_dfork_attr_extents(dip) != 0;
+	if (XFS_DFORK_BOFF(dip) >= XFS_LITINO(sc->mp))
+		return true;
 
 	afork_size = XFS_DFORK_SIZE(dip, sc->mp, XFS_ATTR_FORK);
 	afork_ptr = XFS_DFORK_PTR(dip, XFS_ATTR_FORK);

[PATCH 02/12] xfs: fix data fork block count check in xrep_inode_blockcounts

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM notices that for non-reflink realtime files, the @count output of
xchk_inode_count_blocks includes both the file blocks on the realtime
volume and the bmap btree blocks on the data volume.  Therefore, we must
compare this value to rblocks + dblocks to avoid unnecessary failure.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 2d295fe65776d1 ("xfs: repair inode records")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index de3c915984290b..89cbf599795b2f 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1719,7 +1719,7 @@ xrep_inode_blockcounts(
 		 */
 		;
 	} else if (XFS_IS_REALTIME_INODE(sc->ip)) {
-		if (count >= sc->mp->m_sb.sb_rblocks)
+		if (count >= sc->mp->m_sb.sb_rblocks + sc->mp->m_sb.sb_dblocks)
 			return -EFSCORRUPTED;
 	} else {
 		if (count >= sc->mp->m_sb.sb_dblocks)

[PATCH 03/12] xfs: avoid oversized space allocations when regenerating metadata

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM notices that the btree block preallocation code can be asked to
allocate more than XFS_MAX_BMBT_EXTLEN blocks.  If the allocation
succeeds, the reservation will be stored in a single autoreap record.
Unfortunately, the autoreap code uses deferred frees to get rid of
unused space and that code complains if it's given too much space to get
rid of all once.  Fix this theoretical problem by constraining the
allocation size since it's unlikely that we're going to need that much
space for a btree.  We don't fix the core code because the runtime users
of deferred frees are freeing file space or single btree blocks and we
want to catch problems there.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/newbt.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/newbt.c b/fs/xfs/scrub/newbt.c
index a7554f925b7440..1f28ca1647f8d3 100644
--- a/fs/xfs/scrub/newbt.c
+++ b/fs/xfs/scrub/newbt.c
@@ -280,7 +280,7 @@ xrep_newbt_alloc_ag_blocks(
 			.mp		= mp,
 			.oinfo		= xnr->oinfo,
 			.minlen		= 1,
-			.maxlen		= nr_blocks,
+			.maxlen		= min(XFS_MAX_BMBT_EXTLEN, nr_blocks),
 			.prod		= 1,
 			.resv		= xnr->resv,
 		};
@@ -354,7 +354,7 @@ xrep_newbt_alloc_file_blocks(
 			.mp		= mp,
 			.oinfo		= xnr->oinfo,
 			.minlen		= 1,
-			.maxlen		= nr_blocks,
+			.maxlen		= min(XFS_MAX_BMBT_EXTLEN, nr_blocks),
 			.prod		= 1,
 			.resv		= xnr->resv,
 		};

[PATCH 04/12] xfs: reduce new btree slack factor if free space is negative

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM points out that if the incore fdblocks counter is negative, the
assignment to uint64_t will mask this possibly low space situation and
we don't pack the btree blocks as tightly as we probably ought to.  If
it's the case that there's some free space and fdblocks is too low due
to an error in the superblock field, then we can end up failing btree
rebuilds due to ENOSPC unnecessarily.  This is a corner case, but let's
fix it anyway.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: be408417630427 ("xfs: implement block reservation accounting for btrees we're staging")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/newbt.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/newbt.c b/fs/xfs/scrub/newbt.c
index 1f28ca1647f8d3..2336fe97415120 100644
--- a/fs/xfs/scrub/newbt.c
+++ b/fs/xfs/scrub/newbt.c
@@ -56,7 +56,7 @@ xrep_newbt_estimate_slack(
 {
 	struct xfs_scrub	*sc = xnr->sc;
 	struct xfs_btree_bload	*bload = &xnr->bload;
-	uint64_t		free;
+	int64_t			free;
 	uint64_t		sz;
 
 	/*
@@ -72,6 +72,8 @@ xrep_newbt_estimate_slack(
 		sz = xfs_ag_block_count(sc->mp, pag_agno(sc->sa.pag));
 	} else {
 		free = xfs_sum_freecounter_raw(sc->mp, XC_FREE_BLOCKS);
+		if (free < 0)
+			free = 0;
 		sz = sc->mp->m_sb.sb_dblocks;
 	}
 

[PATCH 05/12] xfs: don't queue rmap update twice when reaping a fork

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that xrep_reap_bmapi_iter calls xfs_bmap_unmap_extent and
xfs_rmap_unmap_extent to remove a mapping from a file fork and its
reverse mapping.  However, xfs_bmap_del_extent_real also queues a
deferred rmap removal log item, which means that we incorrectly queue
two rmap removal operations.  Fix that by removing the extra call and
updating the comment to mention this subtlety that I forgot, having
written the code. :P

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 5befb047b9f4de ("xfs: add the ability to reap entire inode forks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/reap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/reap.c b/fs/xfs/scrub/reap.c
index 528f5ef033d608..9377d687d0cc55 100644
--- a/fs/xfs/scrub/reap.c
+++ b/fs/xfs/scrub/reap.c
@@ -1492,12 +1492,12 @@ xrep_reap_bmapi_iter(
 		/*
 		 * t0: Schedule removal of the mapping from the fork.  We use
 		 * deferred log intents in this function to control the exact
-		 * sequence of metadata updates.
+		 * sequence of metadata updates.  The BUI queues a RUI to
+		 * remove the reverse mapping.
 		 */
 		xfs_bmap_unmap_extent(sc->tp, rs->ip, rs->whichfork, imap);
 		xfs_trans_mod_dquot_byino(sc->tp, rs->ip, XFS_TRANS_DQ_BCOUNT,
 				-(int64_t)imap->br_blockcount);
-		xfs_rmap_unmap_extent(sc->tp, rs->ip, rs->whichfork, imap);
 		return 0;
 	}
 

[PATCH 06/12] xfs: only written file data blocks can be shared

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that xchk_refcountbt_rmap_check (and its rt equivalent)
will count any rmap owner towards the shared count.  However, the only
blocks that can be shared between xfs files are written file data
blocks; and the only blocks that can have single-refcount entries are
COW staging extents.  Fix the checking code.

Cc: <stable@vger.kernel.org> # v4.16
Fixes: dbde19da963706 ("xfs: cross-reference the rmapbt data with the refcountbt")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/refcount.c   |   12 ++++++++++++
 fs/xfs/scrub/rtrefcount.c |   12 ++++++++++++
 2 files changed, 24 insertions(+)


diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
index f8c51d8fbb3da5..a900d13c9e27c0 100644
--- a/fs/xfs/scrub/refcount.c
+++ b/fs/xfs/scrub/refcount.c
@@ -129,6 +129,18 @@ xchk_refcountbt_rmap_check(
 		return 0;
 	}
 
+	/*
+	 * The refcount btree should only have records for written file data
+	 * blocks or for COW staging blocks.
+	 */
+	if ((XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) &&
+	     rec->rm_owner != XFS_RMAP_OWN_COW) ||
+	    (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
+			      XFS_RMAP_UNWRITTEN))) {
+		xchk_btree_xref_set_corrupt(refchk->sc, cur, 0);
+		return 0;
+	}
+
 	if (rec->rm_startblock <= refchk->bno && rm_last >= rc_last) {
 		/*
 		 * The rmap overlaps the refcount record, so we can confirm
diff --git a/fs/xfs/scrub/rtrefcount.c b/fs/xfs/scrub/rtrefcount.c
index 3d916d71a135dc..00da8e98506d63 100644
--- a/fs/xfs/scrub/rtrefcount.c
+++ b/fs/xfs/scrub/rtrefcount.c
@@ -144,6 +144,18 @@ xchk_rtrefcountbt_rmap_check(
 		return 0;
 	}
 
+	/*
+	 * The refcount btree should only have records for written file data
+	 * blocks or for COW staging blocks.
+	 */
+	if ((XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) &&
+	     rec->rm_owner != XFS_RMAP_OWN_COW) ||
+	    (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
+			      XFS_RMAP_UNWRITTEN))) {
+		xchk_btree_xref_set_corrupt(refchk->sc, cur, 0);
+		return 0;
+	}
+
 	if (rec->rm_startblock <= refchk->bno && rm_last >= rc_last) {
 		/*
 		 * The rmap overlaps the refcount record, so we can confirm

[PATCH 07/12] xfs: check for fragments when full-extent sharing matches

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that xchk_refcountbt_process_rmap_fragments returns early
if there are enough rmap records that overlap the space referenced by a
refcountbt record to match the refcount in that refcountbt record.
Unfortunately, it never checks the fragment list in this case, so it
won't find the discrepancy between the rmap and refcount btrees.

Cc: <stable@vger.kernel.org> # v4.16
Fixes: dbde19da963706 ("xfs: cross-reference the rmapbt data with the refcountbt")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/refcount.c   |    5 ++++-
 fs/xfs/scrub/rtrefcount.c |    5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
index a900d13c9e27c0..883c273cb9883f 100644
--- a/fs/xfs/scrub/refcount.c
+++ b/fs/xfs/scrub/refcount.c
@@ -185,8 +185,11 @@ xchk_refcountbt_process_rmap_fragments(
 	xfs_nlink_t			target_nr;
 
 	target_nr = refchk->refcount - refchk->seen;
-	if (target_nr == 0)
+	if (target_nr == 0) {
+		if (!list_empty(&refchk->fragments))
+			refchk->seen = 0;
 		return;
+	}
 
 	/*
 	 * There are (refchk->rc.rc_refcount - refchk->nr refcount)
diff --git a/fs/xfs/scrub/rtrefcount.c b/fs/xfs/scrub/rtrefcount.c
index 00da8e98506d63..c312096e410561 100644
--- a/fs/xfs/scrub/rtrefcount.c
+++ b/fs/xfs/scrub/rtrefcount.c
@@ -200,8 +200,11 @@ xchk_rtrefcountbt_process_rmap_fragments(
 	xfs_nlink_t			target_nr;
 
 	target_nr = refchk->refcount - refchk->seen;
-	if (target_nr == 0)
+	if (target_nr == 0) {
+		if (!list_empty(&refchk->fragments))
+			refchk->seen = 0;
 		return;
+	}
 
 	/*
 	 * There are (refchk->rc.rc_refcount - refchk->nr refcount)

[PATCH 08/12] xfs: don't clear COWEXTSIZE on directories on reflink filesystems

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that xrep_dinode_flags clears the COWEXTSIZE flag
unnecessarily on directories on reflink filesystems.  Directories just
pass the hint along to new children, so it's valid to leave the flag
set.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 2d295fe65776d1 ("xfs: repair inode records")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode_repair.c |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 89cbf599795b2f..4b719ad64ace8c 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -565,6 +565,8 @@ xrep_dinode_flags(
 	 */
 	if (xfs_has_reflink(mp) && S_ISREG(mode))
 		flags2 |= XFS_DIFLAG2_REFLINK;
+	else if (xfs_has_reflink(mp) && S_ISDIR(mode))
+		flags2 &= ~XFS_DIFLAG2_REFLINK;
 	else
 		flags2 &= ~(XFS_DIFLAG2_REFLINK | XFS_DIFLAG2_COWEXTSIZE);
 	if (!xfs_has_bigtime(mp))

[PATCH 09/12] xfs: clear set[ug]id mode bits when zapping attr fork

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM observes that zapping a file's attr fork could result in ACLs
being removed from the file.  Because we have no idea what we're
deleting, we should clear the setuid and setgid bits in addition to the
access mode bits so that nobody can access the file.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: e744cef2060559 ("xfs: zap broken inode forks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode_repair.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 4b719ad64ace8c..25e71e7d472f36 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1313,7 +1313,12 @@ xrep_dinode_zap_afork(
 	 */
 	if (dip->di_format != XFS_DINODE_FMT_BTREE)
 		dip->di_forkoff = 0;
-	dip->di_mode = cpu_to_be16(mode & ~0777);
+
+	/*
+	 * Zapping the attr fork could remove ACLs, so we clear suid/sgid and
+	 * all access mode bits.
+	 */
+	dip->di_mode = cpu_to_be16(mode & ~(S_ISUID | S_ISGID | 0777));
 	dip->di_uid = 0;
 	dip->di_gid = 0;
 }

[PATCH 10/12] xfs: opportunistically skip btree block checks

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM notices that both callers of xchk_btree_get_block will return
early if *pblock is NULL after a successful return.  However,
xchk_btree_get_block itself can return 0 after flagging a corruption.
There's no point in continuing after we've found a corruption, so null
the pointer out.

Cc: <stable@vger.kernel.org> # v4.15
Fixes: cc3e0948d2686f ("xfs: scrub the shape of a metadata btree")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/btree.c |    1 +
 1 file changed, 1 insertion(+)


diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index 786b1e7c4dc37f..43b81432640777 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -602,6 +602,7 @@ xchk_btree_get_block(
 	xfs_btree_get_block(bs->cur, level, pbp);
 	if (__xfs_btree_check_block(bs->cur, *pblock, level, *pbp)) {
 		xchk_btree_set_corrupt(bs->sc, bs->cur, level);
+		*pblock = NULL;
 		return 0;
 	}
 	if (*pbp)

[PATCH 11/12] xfs: always report scrub errors in tracepoints

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM complains that the error processing functions in scrub will
sometimes zero out *error before recording a tracepoint.  This makes
diagnosing problems harder, so let's fix that.

Cc: <stable@vger.kernel.org> # v4.15
Fixes: 4700d22980d459 ("xfs: create helpers to record and deal with scrub problems")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/btree.c   |    8 +++++++-
 fs/xfs/scrub/common.c  |    7 +++++--
 fs/xfs/scrub/dabtree.c |    6 +++++-
 3 files changed, 17 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index 43b81432640777..ce8c9c1aaa3102 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -46,8 +46,14 @@ __xchk_btree_process_error(
 	case -ENODATA:
 		/* Note the badness but don't abort. */
 		sc->sm->sm_flags |= errflag;
+		if (cur->bc_ops->type == XFS_BTREE_TYPE_INODE)
+			trace_xchk_ifork_btree_op_error(sc, cur, level,
+					*error, ret_ip);
+		else
+			trace_xchk_btree_op_error(sc, cur, level,
+					*error, ret_ip);
 		*error = 0;
-		fallthrough;
+		break;
 	default:
 		if (cur->bc_ops->type == XFS_BTREE_TYPE_INODE)
 			trace_xchk_ifork_btree_op_error(sc, cur, level,
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index e5233e31abb750..a61895872072d1 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -107,8 +107,9 @@ __xchk_process_error(
 	case -ENODATA:
 		/* Note the badness but don't abort. */
 		sc->sm->sm_flags |= errflag;
+		trace_xchk_op_error(sc, agno, bno, *error, ret_ip);
 		*error = 0;
-		fallthrough;
+		break;
 	default:
 		trace_xchk_op_error(sc, agno, bno, *error, ret_ip);
 		break;
@@ -183,8 +184,10 @@ __xchk_fblock_process_error(
 	case -ENODATA:
 		/* Note the badness but don't abort. */
 		sc->sm->sm_flags |= errflag;
+		trace_xchk_file_op_error(sc, whichfork, offset, *error,
+				ret_ip);
 		*error = 0;
-		fallthrough;
+		break;
 	default:
 		trace_xchk_file_op_error(sc, whichfork, offset, *error,
 				ret_ip);
diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
index bbf6ece58828ca..02c8544d891861 100644
--- a/fs/xfs/scrub/dabtree.c
+++ b/fs/xfs/scrub/dabtree.c
@@ -49,8 +49,12 @@ xchk_da_process_error(
 	case -ENODATA:
 		/* Note the badness but don't abort. */
 		sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
+		trace_xchk_file_op_error(sc, ds->dargs.whichfork,
+				xfs_dir2_da_to_db(ds->dargs.geo,
+					ds->state->path.blk[level].blkno),
+				*error, __return_address);
 		*error = 0;
-		fallthrough;
+		break;
 	default:
 		trace_xchk_file_op_error(sc, ds->dargs.whichfork,
 				xfs_dir2_da_to_db(ds->dargs.geo,

[PATCH 12/12] xfs: don't scan across metadata/regular directory trees for dirents

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM points out that the directory repair code should never scan files
in the regular directory tree when fixing a metadata directory and
vice-versa.  Amend the scan predicate to check this condition.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: 4f3d4dd1b04b2b ("xfs: define the on-disk format for the metadir feature")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir_repair.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 2cfcf1c35679ea..4a0b912b657696 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1189,7 +1189,9 @@ xrep_dir_want_scan(
 	struct xrep_dir		*rd,
 	const struct xfs_inode	*ip)
 {
-	return ip != rd->sc->ip && !xrep_is_tempfile(ip);
+	return ip != rd->sc->ip &&
+	       xfs_is_metadir_inode(ip) == xfs_is_metadir_inode(rd->sc->ip) &&
+	       !xrep_is_tempfile(ip);
 }
 
 /*

Re: [PATCH 01/12] xfs: don't overrun incore inode when cleaning attr forks

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:55:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xrep_dinode_check_afork doesn't check for a
> di_forkoff value that is so large that the attr fork would begin after
> the end of the inode space.  This causes afork_{ptr,size} to be
> nonsense.  What we really ought to do here is zap the attr fork.

Looks good:

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

Re: [PATCH 02/12] xfs: fix data fork block count check in xrep_inode_blockcounts

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:56:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM notices that for non-reflink realtime files, the @count output of
> xchk_inode_count_blocks includes both the file blocks on the realtime
> volume and the bmap btree blocks on the data volume.  Therefore, we must
> compare this value to rblocks + dblocks to avoid unnecessary failure.

Looks good:

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

Re: [PATCH 03/12] xfs: avoid oversized space allocations when regenerating metadata

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 04/12] xfs: reduce new btree slack factor if free space is negative

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:56:46PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that if the incore fdblocks counter is negative, the
> assignment to uint64_t will mask this possibly low space situation and
> we don't pack the btree blocks as tightly as we probably ought to.  If
> it's the case that there's some free space and fdblocks is too low due
> to an error in the superblock field, then we can end up failing btree
> rebuilds due to ENOSPC unnecessarily.  This is a corner case, but let's
> fix it anyway.

Should thi simply use xfs_sum_freecounter instead of
xfs_sum_freecounter_raw as the former takes care of the issue
in the more "normal" way?

Re: [PATCH 05/12] xfs: don't queue rmap update twice when reaping a fork

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:57:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xrep_reap_bmapi_iter calls xfs_bmap_unmap_extent and
> xfs_rmap_unmap_extent to remove a mapping from a file fork and its
> reverse mapping.  However, xfs_bmap_del_extent_real also queues a
> deferred rmap removal log item, which means that we incorrectly queue
> two rmap removal operations.  Fix that by removing the extra call and
> updating the comment to mention this subtlety that I forgot, having
> written the code. :P

Looks good:

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

Re: [PATCH 06/12] xfs: only written file data blocks can be shared

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:57:18PM -0700, Darrick J. Wong wrote:
> +	/*
> +	 * The refcount btree should only have records for written file data
> +	 * blocks or for COW staging blocks.
> +	 */
> +	if ((XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) &&
> +	     rec->rm_owner != XFS_RMAP_OWN_COW) ||
> +	    (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
> +			      XFS_RMAP_UNWRITTEN))) {
> +		xchk_btree_xref_set_corrupt(refchk->sc, cur, 0);
> +		return 0;
> +	}

> +	/*
> +	 * The refcount btree should only have records for written file data
> +	 * blocks or for COW staging blocks.
> +	 */
> +	if ((XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) &&
> +	     rec->rm_owner != XFS_RMAP_OWN_COW) ||
> +	    (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
> +			      XFS_RMAP_UNWRITTEN))) {
> +		xchk_btree_xref_set_corrupt(refchk->sc, cur, 0);
> +		return 0;
> +	}

Can we have a shared helper for this conditional?

Otherwise this looks good.

Re: [PATCH 07/12] xfs: check for fragments when full-extent sharing matches

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 08/12] xfs: don't clear COWEXTSIZE on directories on reflink filesystems

[Christoph Hellwig]

On Sun, Oct 11, 2026 at 10:57:49PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xrep_dinode_flags clears the COWEXTSIZE flag
> unnecessarily on directories on reflink filesystems.  Directories just
> pass the hint along to new children, so it's valid to leave the flag
> set.

Looks good:

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

Sashiko complains this could corrupt v2 inodes, but IIRC we don't
support online repair on < v5 file sytems, so this should not happen.
Or are we missing a check and accidentally allow repair on v4 file
systems?

Re: [PATCH 09/12] xfs: clear set[ug]id mode bits when zapping attr fork

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 11/12] xfs: always report scrub errors in tracepoints

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 12/12] xfs: don't scan across metadata/regular directory trees for dirents

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 04/12] xfs: reduce new btree slack factor if free space is negative

[Darrick J. Wong]

On Mon, Oct 12, 2026 at 06:08:09AM -0700, Christoph Hellwig wrote:
> On Sun, Oct 11, 2026 at 10:56:46PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM points out that if the incore fdblocks counter is negative, the
> > assignment to uint64_t will mask this possibly low space situation and
> > we don't pack the btree blocks as tightly as we probably ought to.  If
> > it's the case that there's some free space and fdblocks is too low due
> > to an error in the superblock field, then we can end up failing btree
> > rebuilds due to ENOSPC unnecessarily.  This is a corner case, but let's
> > fix it anyway.
> 
> Should thi simply use xfs_sum_freecounter instead of
> xfs_sum_freecounter_raw as the former takes care of the issue
> in the more "normal" way?

Heh, yes.  I forgot that existed :(

Fixed, thanks!

--D

Re: [PATCH 06/12] xfs: only written file data blocks can be shared

[Darrick J. Wong]

On Mon, Oct 12, 2026 at 06:09:16AM -0700, Christoph Hellwig wrote:
> On Sun, Oct 11, 2026 at 10:57:18PM -0700, Darrick J. Wong wrote:
> > +	/*
> > +	 * The refcount btree should only have records for written file data
> > +	 * blocks or for COW staging blocks.
> > +	 */
> > +	if ((XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) &&
> > +	     rec->rm_owner != XFS_RMAP_OWN_COW) ||
> > +	    (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
> > +			      XFS_RMAP_UNWRITTEN))) {
> > +		xchk_btree_xref_set_corrupt(refchk->sc, cur, 0);
> > +		return 0;
> > +	}
> 
> > +	/*
> > +	 * The refcount btree should only have records for written file data
> > +	 * blocks or for COW staging blocks.
> > +	 */
> > +	if ((XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) &&
> > +	     rec->rm_owner != XFS_RMAP_OWN_COW) ||
> > +	    (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
> > +			      XFS_RMAP_UNWRITTEN))) {
> > +		xchk_btree_xref_set_corrupt(refchk->sc, cur, 0);
> > +		return 0;
> > +	}
> 
> Can we have a shared helper for this conditional?
> 
> Otherwise this looks good.

rtrefcount.c goes away entirely with sandeen's series in 7.4-next to
combine the two scrubbers, so in the end there won't be this
duplication.

--D

[PATCH 02/12] xfs: fix data fork block count check in xrep_inode_blockcounts

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM notices that for non-reflink realtime files, the @count output of
xchk_inode_count_blocks includes both the file blocks on the realtime
volume and the bmap btree blocks on the data volume.  Therefore, we must
compare this value to rblocks + dblocks to avoid unnecessary failure.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 2d295fe65776d1 ("xfs: repair inode records")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/inode_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index de3c915984290b..89cbf599795b2f 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1719,7 +1719,7 @@ xrep_inode_blockcounts(
 		 */
 		;
 	} else if (XFS_IS_REALTIME_INODE(sc->ip)) {
-		if (count >= sc->mp->m_sb.sb_rblocks)
+		if (count >= sc->mp->m_sb.sb_rblocks + sc->mp->m_sb.sb_dblocks)
 			return -EFSCORRUPTED;
 	} else {
 		if (count >= sc->mp->m_sb.sb_dblocks)