[PATCHSET] xfs: LLM-inspired bug fixes, part 21

[PATCHSET] xfs: LLM-inspired bug fixes, part 21

[Darrick J. Wong]

Hi all,

Here's a twenty-first 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-21
---
Commits in this patchset:
 * xfs: check for termination during rmap btree walk
 * xfs: don't subtract excess usage twice when computing agf_btreeblks
 * xfs: abort directory tree repair whenever dl->aborted is set
 * xfs: reattach dquots after changing inode ids
 * xfs: never reset the default quota timer
 * xfs: fix correction of unwritten extents in quota file data fork
 * xfs: always report inode repair setup failures
 * xfs: range-check bc_levels array access in scrub tracepoints
 * xfs: reject absurdly large xattr value lengths in xchk_xattr_actor
 * xfs: check for buffer overruns when examining local/remote xattr entries
 * xfs: clarify various parts of the bitmap API
 * xfs: complain loudly when dirtree checker finds broken links
---
 fs/xfs/scrub/repair.h         |    2 +-
 fs/xfs/scrub/trace.h          |   10 +++++-----
 fs/xfs/scrub/alloc_repair.c   |   11 +++++------
 fs/xfs/scrub/attr.c           |   18 ++++++++++++++++++
 fs/xfs/scrub/attr_repair.c    |   16 ++++++++++++++--
 fs/xfs/scrub/bitmap.c         |   20 ++++++++++++++++++++
 fs/xfs/scrub/dirtree.c        |    3 ++-
 fs/xfs/scrub/dirtree_repair.c |    9 +++++++++
 fs/xfs/scrub/inode.c          |    2 +-
 fs/xfs/scrub/inode_repair.c   |   36 +++++++++++++++++++++++++++++++++---
 fs/xfs/scrub/parent_repair.c  |    4 ++++
 fs/xfs/scrub/quota_repair.c   |   21 ++++++++++++++++++++-
 fs/xfs/scrub/trace.c          |   10 ++++++++++
 13 files changed, 142 insertions(+), 20 deletions(-)

[PATCH 01/12] xfs: check for termination during rmap btree walk

[Darrick J. Wong]

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

LOLLM complains that we should check for a fatal signal *every* time we
walk a record in the rmap btree as part of rebuilding the free space
btrees.  Fix that, and improve responsiveness to ^C of xfs_scrub.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 4bdfd7d15747b1 ("xfs: repair free space btrees")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/alloc_repair.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index 6e835240536181..9a09a9ea9c3089 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -224,7 +224,10 @@ xrep_abt_walk_rmap(
 	void				*priv)
 {
 	struct xrep_abt			*ra = priv;
-	int				error;
+	int				error = 0;
+
+	if (xchk_should_terminate(ra->sc, &error))
+		return error;
 
 	/* Record all the OWN_AG blocks... */
 	if (rec->rm_owner == XFS_RMAP_OWN_AG) {

[PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks

[Darrick J. Wong]

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

LOLLM noticed that the free space btree repair code calculates
agf_btreeblks from the number of blocks we wrote to rebuild the free
space btrees (minus the roots).  That's correct.  However, it observes
that we subtract the "excess" reservation, which is not correct.  Fix
this accounting problem.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 4bdfd7d15747b1 ("xfs: repair free space btrees")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/alloc_repair.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)


diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index 9a09a9ea9c3089..05f27c927adfb9 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -660,15 +660,11 @@ xrep_abt_reset_counters(
 
 	/*
 	 * Compute the contribution to agf_btreeblks for the new free space
-	 * btrees.  This is the computed btree size minus anything we didn't
-	 * use.
+	 * btrees.  This is the computed btree size minus the root block.
 	 */
 	freesp_btreeblks += ra->new_bnobt.bload.nr_blocks - 1;
 	freesp_btreeblks += ra->new_cntbt.bload.nr_blocks - 1;
 
-	freesp_btreeblks -= xrep_newbt_unused_blocks(&ra->new_bnobt);
-	freesp_btreeblks -= xrep_newbt_unused_blocks(&ra->new_cntbt);
-
 	/*
 	 * The AGF header contains extra information related to the free space
 	 * btrees, so we must update those fields here.

[PATCH 03/12] xfs: abort directory tree repair whenever dl->aborted is set

[Darrick J. Wong]

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

LOLLM observed that if the live update hooks for the directory tree
repair code ever fail, they'll set dl->aborted, but then the main repair
code path won't check it to exit early.  This can cause incorrect
repairs, so let's check for hook errors and abort the repair.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 3f31406aef493b ("xfs: fix corruptions in the directory tree")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dirtree_repair.c |    9 +++++++++
 1 file changed, 9 insertions(+)


diff --git a/fs/xfs/scrub/dirtree_repair.c b/fs/xfs/scrub/dirtree_repair.c
index 717ebbe4f91916..1de4d9f1b61657 100644
--- a/fs/xfs/scrub/dirtree_repair.c
+++ b/fs/xfs/scrub/dirtree_repair.c
@@ -575,6 +575,8 @@ xrep_dirtree_delete_path(
 
 	if (!error && dl->stale)
 		error = -ESTALE;
+	if (!error && dl->aborted)
+		error = -EIO;
 	return error;
 }
 
@@ -720,6 +722,11 @@ xrep_dirtree_adopt(
 		error = -ESTALE;
 		goto out_trans;
 	}
+	if (dl->aborted) {
+		mutex_unlock(&dl->lock);
+		error = -EIO;
+		goto out_trans;
+	}
 	error = xrep_dirtree_create_adoption_path(dl);
 	mutex_unlock(&dl->lock);
 	if (error)
@@ -780,6 +787,8 @@ xrep_dirtree_move_to_orphanage(
 
 	if (!error && dl->stale)
 		error = -ESTALE;
+	if (!error && dl->aborted)
+		error = -EIO;
 	return error;
 }
 

[PATCH 04/12] xfs: reattach dquots after changing inode ids

[Darrick J. Wong]

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

LOLLM noticed that xrep_inode_ids can change the user, group, or project
id of any given file.  When this happens, we need to detach any existing
incore dquot and attach the correct ones.

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 |   36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 5ecccc75af2500..f8a7dd03f867cf 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -41,6 +41,8 @@
 #include "xfs_rtgroup.h"
 #include "xfs_rtrmap_btree.h"
 #include "xfs_rtrefcount_btree.h"
+#include "xfs_dquot_item.h"
+#include "xfs_dquot.h"
 #include "scrub/xfs_scrub.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
@@ -1748,6 +1750,31 @@ xrep_inode_blockcounts(
 	return 0;
 }
 
+static void
+xrep_inode_switch_dquot(
+	struct xfs_scrub	*sc,
+	xfs_dqtype_t		type,
+	struct xfs_dquot	**dqp)
+{
+#ifdef CONFIG_XFS_QUOTA
+	/*
+	 * We changed one of the file IDs, so we must detach the dquots to
+	 * prevent further changes from being accounted to them, and try to
+	 * attach the dquot for the new id to absorb any other block usage
+	 * changes.  Schedule a quotacheck so the accounting discrepancies will
+	 * get fixed eventually.
+	 */
+	xrep_force_quotacheck(sc, type);
+
+	if (*dqp != NULL) {
+		xfs_qm_dqrele(*dqp);
+		*dqp = NULL;
+	}
+
+	xfs_qm_dqget_inode(sc->ip, type, false, dqp);
+#endif
+}
+
 /* Check for invalid uid/gid/prid. */
 STATIC void
 xrep_inode_ids(
@@ -1761,21 +1788,24 @@ xrep_inode_ids(
 		i_uid_write(VFS_I(sc->ip), 0);
 		dirty = true;
 		if (XFS_IS_UQUOTA_ON(sc->mp))
-			xrep_force_quotacheck(sc, XFS_DQTYPE_USER);
+			xrep_inode_switch_dquot(sc, XFS_DQTYPE_USER,
+					&sc->ip->i_udquot);
 	}
 
 	if (!gid_valid(VFS_I(sc->ip)->i_gid)) {
 		i_gid_write(VFS_I(sc->ip), 0);
 		dirty = true;
 		if (XFS_IS_GQUOTA_ON(sc->mp))
-			xrep_force_quotacheck(sc, XFS_DQTYPE_GROUP);
+			xrep_inode_switch_dquot(sc, XFS_DQTYPE_GROUP,
+					&sc->ip->i_gdquot);
 	}
 
 	if (sc->ip->i_projid == -1U) {
 		sc->ip->i_projid = 0;
 		dirty = true;
 		if (XFS_IS_PQUOTA_ON(sc->mp))
-			xrep_force_quotacheck(sc, XFS_DQTYPE_PROJ);
+			xrep_inode_switch_dquot(sc, XFS_DQTYPE_PROJ,
+					&sc->ip->i_pdquot);
 	}
 
 	/* strip setuid/setgid if we touched any of the ids */

[PATCH 05/12] xfs: never reset the default quota timer

[Darrick J. Wong]

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

LOLLM notices that xrep_quota_fix_timer doesn't check the dquot id,
which means that it can inadvertently reset the default quota timer when
we really meant to ensure that a (non-root) quota overage timeout has a
sane timestamp.  Fix that.

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


diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
index a37fcd3fb6629b..2b695e5b34177c 100644
--- a/fs/xfs/scrub/quota_repair.c
+++ b/fs/xfs/scrub/quota_repair.c
@@ -272,6 +272,9 @@ xrep_quota_fix_timer(
 	time64_t		new_timer;
 	uint32_t		t;
 
+	/* never reset the default quota timer */
+	if (ddq->d_id == 0)
+		return;
 	if (!soft || count <= soft || *timer != 0)
 		return;
 

[PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork

[Darrick J. Wong]

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

LOLLM noticed a couple of problems with the code that fixes unwritten
mappings in the quota file's data fork: first, we should zero the ondisk
contents as part of converting them to "unwritten" just in case we crash
shortly after the conversion.  Second, and more importantly, it noticed
that we fail to resynchronize the iext tree cursor after modifying the
iext tree.  If the tree shape changes, we could be in a world of hurt.
Third, we should force a quotacheck if we reset an ondisk dquot.  Fix
all issues.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: a5b91555403e3a ("xfs: repair quotas")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/quota_repair.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
index 2b695e5b34177c..5ee1ec82fc2fc8 100644
--- a/fs/xfs/scrub/quota_repair.c
+++ b/fs/xfs/scrub/quota_repair.c
@@ -431,6 +431,7 @@ xrep_quota_data_fork(
 	xfs_fsblock_t		fsbno;
 	bool			truncate = false;
 	bool			joined = false;
+	bool			need_quotacheck = false;
 	int			error = 0;
 
 	error = xrep_metadata_inode_forks(sc);
@@ -464,7 +465,8 @@ xrep_quota_data_fork(
 
 			error = xfs_bmapi_write(sc->tp, sc->ip,
 					irec.br_startoff, irec.br_blockcount,
-					XFS_BMAPI_CONVERT, 0, &nrec, &nmap);
+					XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO, 0,
+					&nrec, &nmap);
 			if (error)
 				goto out;
 			ASSERT(nrec.br_startoff == irec.br_startoff);
@@ -473,9 +475,23 @@ xrep_quota_data_fork(
 			error = xfs_defer_finish(&sc->tp);
 			if (error)
 				goto out;
+
+			need_quotacheck = true;
+
+			/*
+			 * Reset cursor to the end of the extent we just
+			 * converted because bmapi write could have merged
+			 * records.
+			 */
+			if (!xfs_iext_lookup_extent(sc->ip, ifp,
+					irec.br_startoff + irec.br_blockcount - 1,
+					&icur, &irec))
+				break;
 		}
 	}
 
+	if (need_quotacheck)
+		xrep_force_quotacheck(sc, dqtype);
 	if (!joined) {
 		xfs_trans_ijoin(sc->tp, sc->ip, 0);
 		joined = true;

[PATCH 07/12] xfs: always report inode repair setup failures

[Darrick J. Wong]

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

LOLLM noticed that xchk_setup_inode silently discards errors being
returned by xrep_setup_inode.  Fix that, and fix the stub macro so that
it actually "returns" a value.

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/repair.h |    2 +-
 fs/xfs/scrub/inode.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
index 2bb125c4f9bf2b..1af568651e20a1 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -239,7 +239,7 @@ xrep_setup_nothing(
 #define xrep_setup_rtrmapbt		xrep_setup_nothing
 #define xrep_setup_rtrefcountbt		xrep_setup_nothing
 
-#define xrep_setup_inode(sc, imap)	((void)0)
+#define xrep_setup_inode(sc, imap)	(0)
 
 static inline int xrep_setup_symlink(struct xfs_scrub *sc, unsigned int *x)
 {
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 46e9bf4a43179e..ac42867acbc4c9 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -212,7 +212,7 @@ xchk_setup_inode(
 	 * make repairs to the ondisk inode buffer.
 	 */
 	if (xchk_could_repair(sc))
-		xrep_setup_inode(sc, &imap);
+		return xrep_setup_inode(sc, &imap);
 	return 0;
 
 out_cancel:

[PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints

[Darrick J. Wong]

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

LOLLM noticed xchk_btree()'s odd habit of passing cur->bc_nlevels to
xchk_btree_ptr_ok when we're checking the pointer to the root block at
the start of the btree scan.  This enables us to skip checking the
pointer for an inode btree's root (since it doesn't have one) but
doesn't handle per-AG btrees quite correctly.  There isn't a problem for
__xfs_btree_check_ptr itself, but the corruption tracepoint naively
walks off the end of bc_level[].  Fix that by adding a helper to do an
implicit range check.

Cc: <stable@vger.kernel.org> # v5.16
Fixes: 6ca444cfd66354 ("xfs: prepare xfs_btree_cur for dynamic cursor heights")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/trace.h |   10 +++++-----
 fs/xfs/scrub/trace.c |   10 ++++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index cb85f75ce10180..6b14b149b6000f 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -577,7 +577,7 @@ TRACE_EVENT(xchk_btree_op_error,
 		__entry->level = level;
 		__entry->agno = XFS_FSB_TO_AGNO(cur->bc_mp, fsbno);
 		__entry->bno = XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno);
-		__entry->ptr = cur->bc_levels[level].ptr;
+		__entry->ptr = xchk_btree_cur_ptr(cur, level);
 		__entry->error = error;
 		__entry->ret_ip = ret_ip;
 	),
@@ -618,7 +618,7 @@ TRACE_EVENT(xchk_ifork_btree_op_error,
 		__entry->type = sc->sm->sm_type;
 		__assign_str(name);
 		__entry->level = level;
-		__entry->ptr = cur->bc_levels[level].ptr;
+		__entry->ptr = xchk_btree_cur_ptr(cur, level);
 		__entry->agno = XFS_FSB_TO_AGNO(cur->bc_mp, fsbno);
 		__entry->bno = XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno);
 		__entry->error = error;
@@ -660,7 +660,7 @@ TRACE_EVENT(xchk_btree_error,
 		__entry->level = level;
 		__entry->agno = XFS_FSB_TO_AGNO(cur->bc_mp, fsbno);
 		__entry->bno = XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno);
-		__entry->ptr = cur->bc_levels[level].ptr;
+		__entry->ptr = xchk_btree_cur_ptr(cur, level);
 		__entry->ret_ip = ret_ip;
 	),
 	TP_printk("dev %d:%d type %s %sbt level %d ptr %d agno 0x%x agbno 0x%x ret_ip %pS",
@@ -700,7 +700,7 @@ TRACE_EVENT(xchk_ifork_btree_error,
 		__entry->level = level;
 		__entry->agno = XFS_FSB_TO_AGNO(cur->bc_mp, fsbno);
 		__entry->bno = XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno);
-		__entry->ptr = cur->bc_levels[level].ptr;
+		__entry->ptr = xchk_btree_cur_ptr(cur, level);
 		__entry->ret_ip = ret_ip;
 	),
 	TP_printk("dev %d:%d ino 0x%llx fork %s type %s %sbt level %d ptr %d agno 0x%x agbno 0x%x ret_ip %pS",
@@ -740,7 +740,7 @@ DECLARE_EVENT_CLASS(xchk_sbtree_class,
 		__entry->bno = XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno);
 		__entry->level = level;
 		__entry->nlevels = cur->bc_nlevels;
-		__entry->ptr = cur->bc_levels[level].ptr;
+		__entry->ptr = xchk_btree_cur_ptr(cur, level);
 	),
 	TP_printk("dev %d:%d type %s %sbt agno 0x%x agbno 0x%x level %d nlevels %d ptr %d",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
diff --git a/fs/xfs/scrub/trace.c b/fs/xfs/scrub/trace.c
index e5efc680651fce..29bcff41302394 100644
--- a/fs/xfs/scrub/trace.c
+++ b/fs/xfs/scrub/trace.c
@@ -53,6 +53,16 @@ xchk_btree_cur_fsbno(
 	return NULLFSBLOCK;
 }
 
+static inline int
+xchk_btree_cur_ptr(
+	const struct xfs_btree_cur	*cur,
+	int				level)
+{
+	if (level >= 0 && level < cur->bc_nlevels)
+		return cur->bc_levels[level].ptr;
+	return -1;
+}
+
 /*
  * We include this last to have the helpers above available for the trace
  * event implementations.

[PATCH 09/12] xfs: reject absurdly large xattr value lengths in xchk_xattr_actor

[Darrick J. Wong]

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

LOLLM suggests that we shouldn't try to allocate absurdly large buffers
for xattr value checks because that will push reclaim very hard, never
succeed, and is pointless because nobody can create xattrs that large.
Since the xattr repair code ignores values larger than XATTR_SIZE_MAX,
we can flag the corruption and return early.

Cc: <stable@vger.kernel.org> # v5.3
Fixes: 6d6ccedd76823c ("xfs: only allocate memory for scrubbing attributes when we need it")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/attr.c          |    6 ++++++
 fs/xfs/scrub/parent_repair.c |    4 ++++
 2 files changed, 10 insertions(+)


diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 7600c711362583..842d65834afbc9 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -216,6 +216,12 @@ xchk_xattr_actor(
 		return -ECANCELED;
 	}
 
+	/* Don't even try to handle a bizarrely large xattr. */
+	if (valuelen > XATTR_SIZE_MAX) {
+		xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, args.blkno);
+		return -ECANCELED;
+	}
+
 	/*
 	 * Try to allocate enough memory to extract the attr value.  If that
 	 * doesn't work, return -EDEADLOCK as a signal to try again with a
diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c
index f643724910f954..6a3c0a190ff995 100644
--- a/fs/xfs/scrub/parent_repair.c
+++ b/fs/xfs/scrub/parent_repair.c
@@ -910,6 +910,10 @@ xrep_parent_fetch_xattr_remote(
 	};
 	int			error;
 
+	/* Don't even try to handle a bizarrely large xattr. */
+	if (valuelen > XATTR_SIZE_MAX)
+		return -EFSCORRUPTED;
+
 	/*
 	 * If we need a larger value buffer, try to allocate one.  If that
 	 * fails, return with -EDEADLOCK to try harder.

[PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries

[Darrick J. Wong]

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

LOLLM noticed that the xattr scrub and repair code can walk off the end
of the buffer if the xattr entry record starts so close to the end of
the buffer that there's not even space for the entry structure itself.
Strengthen the bounds checking here.

Cc: <stable@vger.kernel.org> # v4.15
Fixes: 13791d3b833428 ("xfs: scrub extended attribute leaf space")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/attr.c        |   12 ++++++++++++
 fs/xfs/scrub/attr_repair.c |   16 ++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 842d65834afbc9..4ce7299060fbba 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -335,6 +335,12 @@ xchk_xattr_entry(
 	/* Check the name information. */
 	if (ent->flags & XFS_ATTR_LOCAL) {
 		lentry = xfs_attr3_leaf_name_local(leaf, idx);
+		name_end = (char *)lentry +
+				offsetof(struct xfs_attr_leaf_name_local, nameval[0]);
+		if (name_end > buf_end) {
+			xchk_da_set_corrupt(ds, level);
+			return;
+		}
 		namesize = xfs_attr_leaf_entsize_local(lentry->namelen,
 				be16_to_cpu(lentry->valuelen));
 		name_end = (char *)lentry + namesize;
@@ -342,6 +348,12 @@ xchk_xattr_entry(
 			xchk_da_set_corrupt(ds, level);
 	} else {
 		rentry = xfs_attr3_leaf_name_remote(leaf, idx);
+		name_end = (char *)rentry +
+				offsetof(struct xfs_attr_leaf_name_remote, name[0]);
+		if (name_end > buf_end) {
+			xchk_da_set_corrupt(ds, level);
+			return;
+		}
 		namesize = xfs_attr_leaf_entsize_remote(rentry->namelen);
 		name_end = (char *)rentry + namesize;
 		if (rentry->namelen == 0)
diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
index c7695d4cc8e7cc..25e71b3a14d95d 100644
--- a/fs/xfs/scrub/attr_repair.c
+++ b/fs/xfs/scrub/attr_repair.c
@@ -285,6 +285,7 @@ xrep_xattr_salvage_local_attr(
 	struct xfs_attr_leaf_name_local	*lentry)
 {
 	struct xchk_xattr_buf		*ab = rx->sc->buf;
+	char				*name_end;
 	unsigned char			*value;
 	unsigned int			valuelen;
 	unsigned int			namesize;
@@ -293,10 +294,15 @@ xrep_xattr_salvage_local_attr(
 	 * Decode the leaf local entry format.  If something seems wrong, we
 	 * junk the attribute.
 	 */
+	name_end = (char *)lentry +
+			offsetof(struct xfs_attr_leaf_name_local, nameval[0]);
+	if (name_end > buf_end)
+		return 0;
 	value = &lentry->nameval[lentry->namelen];
 	valuelen = be16_to_cpu(lentry->valuelen);
 	namesize = xfs_attr_leaf_entsize_local(lentry->namelen, valuelen);
-	if ((char *)lentry + namesize > buf_end)
+	name_end = (char *)lentry + namesize;
+	if (name_end > buf_end)
 		return 0;
 	if (!xrep_xattr_want_salvage(rx, ent->flags, lentry->nameval,
 			lentry->namelen, value, valuelen))
@@ -335,6 +341,7 @@ xrep_xattr_salvage_remote_attr(
 		.name			= rentry->name,
 		.valuelen		= be32_to_cpu(rentry->valuelen),
 	};
+	char				*name_end;
 	unsigned int			namesize;
 	int				error;
 
@@ -343,8 +350,13 @@ xrep_xattr_salvage_remote_attr(
 	 * junk the attribute.  Note that we should never find a zero-length
 	 * remote attribute value.
 	 */
+	name_end = (char *)rentry +
+			offsetof(struct xfs_attr_leaf_name_remote, name[0]);
+	if (name_end > buf_end)
+		return 0;
 	namesize = xfs_attr_leaf_entsize_remote(rentry->namelen);
-	if ((char *)rentry + namesize > buf_end)
+	name_end = (char *)rentry + namesize;
+	if (name_end > buf_end)
 		return 0;
 	if (args.valuelen == 0 ||
 	    !xrep_xattr_want_salvage(rx, ent->flags, rentry->name,

[PATCH 11/12] xfs: clarify various parts of the bitmap API

[Darrick J. Wong]

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

LOLLM pointed out some underspecified parts of the xbitmap{32,64} API.
Specifically, the behavior of set/clear/test functions when the length
parameter is zero is not well defined, nor is what happens when someone
tries to disunion a bitmap with itself.  None of these situations are
expected, but we'll define zero-length set and clear to be nops, and the
rest to be checked by assertions.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 6ece924b952262 ("xfs: create separate structures and code for u32 bitmaps")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/bitmap.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)


diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
index 08f216d26ec632..1da99e179baa42 100644
--- a/fs/xfs/scrub/bitmap.c
+++ b/fs/xfs/scrub/bitmap.c
@@ -77,6 +77,9 @@ xbitmap64_clear(
 	struct xbitmap64_node	*new_bn;
 	uint64_t		last = start + len - 1;
 
+	if (!len)
+		return 0;
+
 	while ((bn = xbitmap64_tree_iter_first(&bitmap->xb_root, start, last))) {
 		if (bn->bn_start < start && bn->bn_last > last) {
 			uint64_t	old_last = bn->bn_last;
@@ -127,6 +130,9 @@ xbitmap64_set(
 	uint64_t		last = start + len - 1;
 	int			error;
 
+	if (!len)
+		return 0;
+
 	/* Is this whole range already set? */
 	left = xbitmap64_tree_iter_first(&bitmap->xb_root, start, last);
 	if (left && left->bn_start <= start && left->bn_last >= last)
@@ -223,6 +229,8 @@ xbitmap64_disunion(
 	struct xbitmap64_node	*bn;
 	int			error;
 
+	ASSERT(bitmap != sub);
+
 	if (xbitmap64_empty(bitmap) || xbitmap64_empty(sub))
 		return 0;
 
@@ -287,6 +295,8 @@ xbitmap64_test(
 	struct xbitmap64_node	*bn;
 	uint64_t		last = start + *len - 1;
 
+	ASSERT(*len > 0);
+
 	bn = xbitmap64_tree_iter_first(&bitmap->xb_root, start, last);
 	if (!bn)
 		return false;
@@ -357,6 +367,9 @@ xbitmap32_clear(
 	struct xbitmap32_node	*new_bn;
 	uint32_t		last = start + len - 1;
 
+	if (!len)
+		return 0;
+
 	while ((bn = xbitmap32_tree_iter_first(&bitmap->xb_root, start, last))) {
 		if (bn->bn_start < start && bn->bn_last > last) {
 			uint32_t	old_last = bn->bn_last;
@@ -407,6 +420,9 @@ xbitmap32_set(
 	uint32_t		last = start + len - 1;
 	int			error;
 
+	if (!len)
+		return 0;
+
 	/* Is this whole range already set? */
 	left = xbitmap32_tree_iter_first(&bitmap->xb_root, start, last);
 	if (left && left->bn_start <= start && left->bn_last >= last)
@@ -503,6 +519,8 @@ xbitmap32_disunion(
 	struct xbitmap32_node	*bn;
 	int			error;
 
+	ASSERT(bitmap != sub);
+
 	if (xbitmap32_empty(bitmap) || xbitmap32_empty(sub))
 		return 0;
 
@@ -567,6 +585,8 @@ xbitmap32_test(
 	struct xbitmap32_node	*bn;
 	uint32_t		last = start + *len - 1;
 
+	ASSERT(*len > 0);
+
 	bn = xbitmap32_tree_iter_first(&bitmap->xb_root, start, last);
 	if (!bn)
 		return false;

[PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links

[Darrick J. Wong]

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

LOLLM observed that the directory tree walk code bails out if it finds
broken parent pointers along the way.  However, it flattens the return
code to zero, which means that we can end up in the repair code having
not completed the scan.  We should never proceed with a repair in this
state, so let's mark the scrub incomplete and exit.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 928b721a11789a ("xfs: teach online scrub to find directory tree structure problems")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dirtree.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dirtree.c b/fs/xfs/scrub/dirtree.c
index 887383d2e94104..5d2fee2138439b 100644
--- a/fs/xfs/scrub/dirtree.c
+++ b/fs/xfs/scrub/dirtree.c
@@ -860,7 +860,8 @@ xchk_dirtree_find_paths_to_root(
 				 * A parent pointer of @sc->ip is bad, don't
 				 * bother continuing.
 				 */
-				break;
+				xchk_set_incomplete(sc);
+				return error;
 			}
 			if (error == -ESTALE) {
 				/* This had better be an invalidation. */

Re: [PATCH 01/12] xfs: check for termination during rmap btree walk

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:55:05PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that we should check for a fatal signal *every* time we
> walk a record in the rmap btree as part of rebuilding the free space
> btrees.  Fix that, and improve responsiveness to ^C of xfs_scrub.

Looks good:

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

Re: [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:55:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that the free space btree repair code calculates
> agf_btreeblks from the number of blocks we wrote to rebuild the free
> space btrees (minus the roots).  That's correct.  However, it observes
> that we subtract the "excess" reservation, which is not correct.  Fix
> this accounting problem.

xrep_newbt_unused_blocks is unused now.

Re: [PATCH 03/12] xfs: abort directory tree repair whenever dl->aborted is set

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:55:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM observed that if the live update hooks for the directory tree
> repair code ever fail, they'll set dl->aborted, but then the main repair
> code path won't check it to exit early.  This can cause incorrect
> repairs, so let's check for hook errors and abort the repair.

Looks good:

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

Re: [PATCH 04/12] xfs: reattach dquots after changing inode ids

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:55:52PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xrep_inode_ids can change the user, group, or project
> id of any given file.  When this happens, we need to detach any existing
> incore dquot and attach the correct ones.

Sashiko complains about changing the dquot while attached to the
inode and I think it is right, or rather the issue goes even deeper.
We should not have the dquot attached while changing these identifier
as otherwise things will get really nasty.  I'm not really sure how
to do better here, though.

Re: [PATCH 05/12] xfs: never reset the default quota timer

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:56:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM notices that xrep_quota_fix_timer doesn't check the dquot id,
> which means that it can inadvertently reset the default quota timer when
> we really meant to ensure that a (non-root) quota overage timeout has a
> sane timestamp.  Fix that.

Looks good:

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

Re: [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:56:24PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed a couple of problems with the code that fixes unwritten
> mappings in the quota file's data fork: first, we should zero the ondisk
> contents as part of converting them to "unwritten" just in case we crash
> shortly after the conversion.  Second, and more importantly, it noticed
> that we fail to resynchronize the iext tree cursor after modifying the
> iext tree.  If the tree shape changes, we could be in a world of hurt.
> Third, we should force a quotacheck if we reset an ondisk dquot.  Fix
> all issues.

I don't think the existing code here that mixes raw cursor based
iext access and xfs_bmapi_write is safe, and that is probably also
the root cause of the sashiko complaints.  So we'll probably need
to drop out of the loop, finish and restart from the new offset.

Re: [PATCH 07/12] xfs: always report inode repair setup failures

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:56:39PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that xchk_setup_inode silently discards errors being
> returned by xrep_setup_inode.  Fix that, and fix the stub macro so that
> it actually "returns" a value.

Looks good:

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

Re: [PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:56:55PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed xchk_btree()'s odd habit of passing cur->bc_nlevels to
> xchk_btree_ptr_ok when we're checking the pointer to the root block at
> the start of the btree scan.  This enables us to skip checking the
> pointer for an inode btree's root (since it doesn't have one) but
> doesn't handle per-AG btrees quite correctly.  There isn't a problem for
> __xfs_btree_check_ptr itself, but the corruption tracepoint naively
> walks off the end of bc_level[].  Fix that by adding a helper to do an
> implicit range check.

Looks good:

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

Re: [PATCH 09/12] xfs: reject absurdly large xattr value lengths in xchk_xattr_actor

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:57:11PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM suggests that we shouldn't try to allocate absurdly large buffers
> for xattr value checks because that will push reclaim very hard, never
> succeed, and is pointless because nobody can create xattrs that large.
> Since the xattr repair code ignores values larger than XATTR_SIZE_MAX,
> we can flag the corruption and return early.

Looks good:

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

Re: [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:57:26PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that the xattr scrub and repair code can walk off the end
> of the buffer if the xattr entry record starts so close to the end of
> the buffer that there's not even space for the entry structure itself.
> Strengthen the bounds checking here.
> 
> Cc: <stable@vger.kernel.org> # v4.15
> Fixes: 13791d3b833428 ("xfs: scrub extended attribute leaf space")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/attr.c        |   12 ++++++++++++
>  fs/xfs/scrub/attr_repair.c |   16 ++++++++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index 842d65834afbc9..4ce7299060fbba 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -335,6 +335,12 @@ xchk_xattr_entry(
>  	/* Check the name information. */
>  	if (ent->flags & XFS_ATTR_LOCAL) {
>  		lentry = xfs_attr3_leaf_name_local(leaf, idx);
> +		name_end = (char *)lentry +
> +				offsetof(struct xfs_attr_leaf_name_local, nameval[0]);

Overly long line.

> +		if (name_end > buf_end) {
> +			xchk_da_set_corrupt(ds, level);
> +			return;
> +		}
>  		namesize = xfs_attr_leaf_entsize_local(lentry->namelen,
>  				be16_to_cpu(lentry->valuelen));
>  		name_end = (char *)lentry + namesize;
> @@ -342,6 +348,12 @@ xchk_xattr_entry(
>  			xchk_da_set_corrupt(ds, level);
>  	} else {
>  		rentry = xfs_attr3_leaf_name_remote(leaf, idx);
> +		name_end = (char *)rentry +
> +				offsetof(struct xfs_attr_leaf_name_remote, name[0]);

Same here.

Can you split the local vs remote code into separate helpers as
that would really help with readability here?

Also for this long type names just using typeof might be nicer than
spelling out the type.

Re: [PATCH 11/12] xfs: clarify various parts of the bitmap API

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links

[Christoph Hellwig]

On Wed, Oct 07, 2026 at 10:57:57PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM observed that the directory tree walk code bails out if it finds
> broken parent pointers along the way.  However, it flattens the return
> code to zero, which means that we can end up in the repair code having
> not completed the scan.  We should never proceed with a repair in this
> state, so let's mark the scrub incomplete and exit.

Looks good:

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

although I think Sashiko is right in that more places need the same
treatment.

Re: [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks

[Darrick J. Wong]

On Thu, Oct 08, 2026 at 04:54:12AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 07, 2026 at 10:55:21PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that the free space btree repair code calculates
> > agf_btreeblks from the number of blocks we wrote to rebuild the free
> > space btrees (minus the roots).  That's correct.  However, it observes
> > that we subtract the "excess" reservation, which is not correct.  Fix
> > this accounting problem.
> 
> xrep_newbt_unused_blocks is unused now.

Oh, heh, I'll get rid of it then, thanks for pointing that out.

--D

Re: [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork

[Darrick J. Wong]

On Thu, Oct 08, 2026 at 05:00:05AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 07, 2026 at 10:56:24PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed a couple of problems with the code that fixes unwritten
> > mappings in the quota file's data fork: first, we should zero the ondisk
> > contents as part of converting them to "unwritten" just in case we crash
> > shortly after the conversion.  Second, and more importantly, it noticed
> > that we fail to resynchronize the iext tree cursor after modifying the
> > iext tree.  If the tree shape changes, we could be in a world of hurt.
> > Third, we should force a quotacheck if we reset an ondisk dquot.  Fix
> > all issues.
> 
> I don't think the existing code here that mixes raw cursor based
> iext access and xfs_bmapi_write is safe, and that is probably also
> the root cause of the sashiko complaints.  So we'll probably need
> to drop out of the loop, finish and restart from the new offset.

Oh, yeah.  I hadn't realized that the merging could involve the *next*
extent, in which case we skip right past to the next one.  Instead, I'll
redo the loop like this:

	ifp = xfs_ifork_ptr(sc->ip, XFS_DATA_FORK);
	xfs_iext_first(ifp, &icur);

	while (xfs_iext_get_extent(ifp, &icur, &irec)) {
		...

and then the end of the loop becomes:

			error = xfs_bmapi_write(...);

			/*
			 * Set cursor to the next block past the extent we just
			 * converted because bmapi write could have merged
			 * records.
			 */
			if (!xfs_iext_lookup_extent(sc->ip, ifp,
					irec.br_startoff + irec.br_blockcount,
					&icur, &irec))
				break;
			continue;
		}

		xfs_iext_next(ifp, &icur);
	}

Also I'm pretty sure the xrep_force_quotacheck should go with the same
transaction that captures the xfs_bmapi_write.

--D

Re: [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries

[Darrick J. Wong]

On Thu, Oct 08, 2026 at 05:03:38AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 07, 2026 at 10:57:26PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that the xattr scrub and repair code can walk off the end
> > of the buffer if the xattr entry record starts so close to the end of
> > the buffer that there's not even space for the entry structure itself.
> > Strengthen the bounds checking here.
> > 
> > Cc: <stable@vger.kernel.org> # v4.15
> > Fixes: 13791d3b833428 ("xfs: scrub extended attribute leaf space")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > Assisted-by: LOLLM # finding obvious bugs
> > ---
> >  fs/xfs/scrub/attr.c        |   12 ++++++++++++
> >  fs/xfs/scrub/attr_repair.c |   16 ++++++++++++++--
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> > index 842d65834afbc9..4ce7299060fbba 100644
> > --- a/fs/xfs/scrub/attr.c
> > +++ b/fs/xfs/scrub/attr.c
> > @@ -335,6 +335,12 @@ xchk_xattr_entry(
> >  	/* Check the name information. */
> >  	if (ent->flags & XFS_ATTR_LOCAL) {
> >  		lentry = xfs_attr3_leaf_name_local(leaf, idx);
> > +		name_end = (char *)lentry +
> > +				offsetof(struct xfs_attr_leaf_name_local, nameval[0]);
> 
> Overly long line.
> 
> > +		if (name_end > buf_end) {
> > +			xchk_da_set_corrupt(ds, level);
> > +			return;
> > +		}
> >  		namesize = xfs_attr_leaf_entsize_local(lentry->namelen,
> >  				be16_to_cpu(lentry->valuelen));
> >  		name_end = (char *)lentry + namesize;
> > @@ -342,6 +348,12 @@ xchk_xattr_entry(
> >  			xchk_da_set_corrupt(ds, level);
> >  	} else {
> >  		rentry = xfs_attr3_leaf_name_remote(leaf, idx);
> > +		name_end = (char *)rentry +
> > +				offsetof(struct xfs_attr_leaf_name_remote, name[0]);
> 
> Same here.
> 
> Can you split the local vs remote code into separate helpers as
> that would really help with readability here?

Will do, as a prep patch that comes before this one.

> Also for this long type names just using typeof might be nicer than
> spelling out the type.

Ooh, that works.  I'll fix that too.

--D

Re: [PATCH 04/12] xfs: reattach dquots after changing inode ids

[Darrick J. Wong]

On Thu, Oct 08, 2026 at 04:57:03AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 07, 2026 at 10:55:52PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that xrep_inode_ids can change the user, group, or project
> > id of any given file.  When this happens, we need to detach any existing
> > incore dquot and attach the correct ones.
> 
> Sashiko complains about changing the dquot while attached to the
> inode and I think it is right, or rather the issue goes even deeper.
> We should not have the dquot attached while changing these identifier
> as otherwise things will get really nasty.  I'm not really sure how
> to do better here, though.

Yeah.... as Sashiko points out, the dquots might be attached to the
transaction, so dqdetaching them with a transaction held is baaaad.
The dquot detach and reattach have to get done at the end, after we've
committed all the inode updates.  Will update this patch and resend it.

--D

Re: [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links

[Darrick J. Wong]

On Thu, Oct 08, 2026 at 05:04:44AM -0700, Christoph Hellwig wrote:
> On Wed, Oct 07, 2026 at 10:57:57PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM observed that the directory tree walk code bails out if it finds
> > broken parent pointers along the way.  However, it flattens the return
> > code to zero, which means that we can end up in the repair code having
> > not completed the scan.  We should never proceed with a repair in this
> > state, so let's mark the scrub incomplete and exit.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> although I think Sashiko is right in that more places need the same
> treatment.

Aha, yes, in the branch that flattens ENOSR/ELRNG/EFSCORRUPTED to zero.
Will fix that.

--D