* [PATCH 01/12] xfs: check for termination during rmap btree walk
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
@ 2026-10-08 5:55 ` Darrick J. Wong
2026-10-08 11:52 ` Christoph Hellwig
2026-10-08 5:55 ` [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks Darrick J. Wong
` (10 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:55 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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) {
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
2026-10-08 5:55 ` [PATCH 01/12] xfs: check for termination during rmap btree walk Darrick J. Wong
@ 2026-10-08 5:55 ` Darrick J. Wong
2026-10-08 11:54 ` Christoph Hellwig
2026-10-08 5:55 ` [PATCH 03/12] xfs: abort directory tree repair whenever dl->aborted is set Darrick J. Wong
` (9 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:55 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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.
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks
2026-10-08 5:55 ` [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks Darrick J. Wong
@ 2026-10-08 11:54 ` Christoph Hellwig
2026-10-08 15:44 ` Darrick J. Wong
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2026-10-08 11:54 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, stable, linux-xfs
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks
2026-10-08 11:54 ` Christoph Hellwig
@ 2026-10-08 15:44 ` Darrick J. Wong
0 siblings, 0 replies; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 15:44 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: cem, stable, linux-xfs
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 03/12] xfs: abort directory tree repair whenever dl->aborted is set
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
2026-10-08 5:55 ` [PATCH 01/12] xfs: check for termination during rmap btree walk Darrick J. Wong
2026-10-08 5:55 ` [PATCH 02/12] xfs: don't subtract excess usage twice when computing agf_btreeblks Darrick J. Wong
@ 2026-10-08 5:55 ` Darrick J. Wong
2026-10-08 11:54 ` Christoph Hellwig
2026-10-08 5:55 ` [PATCH 04/12] xfs: reattach dquots after changing inode ids Darrick J. Wong
` (8 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:55 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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;
}
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 04/12] xfs: reattach dquots after changing inode ids
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (2 preceding siblings ...)
2026-10-08 5:55 ` [PATCH 03/12] xfs: abort directory tree repair whenever dl->aborted is set Darrick J. Wong
@ 2026-10-08 5:55 ` Darrick J. Wong
2026-10-08 11:57 ` Christoph Hellwig
2026-10-08 5:56 ` [PATCH 05/12] xfs: never reset the default quota timer Darrick J. Wong
` (7 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:55 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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 */
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 04/12] xfs: reattach dquots after changing inode ids
2026-10-08 5:55 ` [PATCH 04/12] xfs: reattach dquots after changing inode ids Darrick J. Wong
@ 2026-10-08 11:57 ` Christoph Hellwig
2026-10-08 17:20 ` Darrick J. Wong
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2026-10-08 11:57 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, stable, linux-xfs
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 04/12] xfs: reattach dquots after changing inode ids
2026-10-08 11:57 ` Christoph Hellwig
@ 2026-10-08 17:20 ` Darrick J. Wong
0 siblings, 0 replies; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 17:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: cem, stable, linux-xfs
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 05/12] xfs: never reset the default quota timer
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (3 preceding siblings ...)
2026-10-08 5:55 ` [PATCH 04/12] xfs: reattach dquots after changing inode ids Darrick J. Wong
@ 2026-10-08 5:56 ` Darrick J. Wong
2026-10-08 11:57 ` Christoph Hellwig
2026-10-08 5:56 ` [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork Darrick J. Wong
` (6 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:56 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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;
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (4 preceding siblings ...)
2026-10-08 5:56 ` [PATCH 05/12] xfs: never reset the default quota timer Darrick J. Wong
@ 2026-10-08 5:56 ` Darrick J. Wong
2026-10-08 12:00 ` Christoph Hellwig
2026-10-08 5:56 ` [PATCH 07/12] xfs: always report inode repair setup failures Darrick J. Wong
` (5 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:56 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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;
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork
2026-10-08 5:56 ` [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork Darrick J. Wong
@ 2026-10-08 12:00 ` Christoph Hellwig
2026-10-08 15:59 ` Darrick J. Wong
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2026-10-08 12:00 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, stable, linux-xfs
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork
2026-10-08 12:00 ` Christoph Hellwig
@ 2026-10-08 15:59 ` Darrick J. Wong
0 siblings, 0 replies; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 15:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: cem, stable, linux-xfs
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 07/12] xfs: always report inode repair setup failures
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (5 preceding siblings ...)
2026-10-08 5:56 ` [PATCH 06/12] xfs: fix correction of unwritten extents in quota file data fork Darrick J. Wong
@ 2026-10-08 5:56 ` Darrick J. Wong
2026-10-08 12:00 ` Christoph Hellwig
2026-10-08 5:56 ` [PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints Darrick J. Wong
` (4 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:56 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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:
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (6 preceding siblings ...)
2026-10-08 5:56 ` [PATCH 07/12] xfs: always report inode repair setup failures Darrick J. Wong
@ 2026-10-08 5:56 ` Darrick J. Wong
2026-10-08 12:01 ` Christoph Hellwig
2026-10-08 5:57 ` [PATCH 09/12] xfs: reject absurdly large xattr value lengths in xchk_xattr_actor Darrick J. Wong
` (3 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:56 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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.
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints
2026-10-08 5:56 ` [PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints Darrick J. Wong
@ 2026-10-08 12:01 ` Christoph Hellwig
0 siblings, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2026-10-08 12:01 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, stable, linux-xfs
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>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 09/12] xfs: reject absurdly large xattr value lengths in xchk_xattr_actor
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (7 preceding siblings ...)
2026-10-08 5:56 ` [PATCH 08/12] xfs: range-check bc_levels array access in scrub tracepoints Darrick J. Wong
@ 2026-10-08 5:57 ` Darrick J. Wong
2026-10-08 12:01 ` Christoph Hellwig
2026-10-08 5:57 ` [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries Darrick J. Wong
` (2 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:57 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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.
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (8 preceding siblings ...)
2026-10-08 5:57 ` [PATCH 09/12] xfs: reject absurdly large xattr value lengths in xchk_xattr_actor Darrick J. Wong
@ 2026-10-08 5:57 ` Darrick J. Wong
2026-10-08 12:03 ` Christoph Hellwig
2026-10-08 5:57 ` [PATCH 11/12] xfs: clarify various parts of the bitmap API Darrick J. Wong
2026-10-08 5:57 ` [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links Darrick J. Wong
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:57 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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,
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries
2026-10-08 5:57 ` [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries Darrick J. Wong
@ 2026-10-08 12:03 ` Christoph Hellwig
2026-10-08 16:17 ` Darrick J. Wong
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2026-10-08 12:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, stable, linux-xfs
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.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries
2026-10-08 12:03 ` Christoph Hellwig
@ 2026-10-08 16:17 ` Darrick J. Wong
0 siblings, 0 replies; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 16:17 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: cem, stable, linux-xfs
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 11/12] xfs: clarify various parts of the bitmap API
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (9 preceding siblings ...)
2026-10-08 5:57 ` [PATCH 10/12] xfs: check for buffer overruns when examining local/remote xattr entries Darrick J. Wong
@ 2026-10-08 5:57 ` Darrick J. Wong
2026-10-08 12:04 ` Christoph Hellwig
2026-10-08 5:57 ` [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links Darrick J. Wong
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:57 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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;
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links
2026-10-08 5:55 [PATCHSET] xfs: LLM-inspired bug fixes, part 21 Darrick J. Wong
` (10 preceding siblings ...)
2026-10-08 5:57 ` [PATCH 11/12] xfs: clarify various parts of the bitmap API Darrick J. Wong
@ 2026-10-08 5:57 ` Darrick J. Wong
2026-10-08 12:04 ` Christoph Hellwig
11 siblings, 1 reply; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 5:57 UTC (permalink / raw)
To: djwong, cem; +Cc: stable, linux-xfs
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. */
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links
2026-10-08 5:57 ` [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links Darrick J. Wong
@ 2026-10-08 12:04 ` Christoph Hellwig
2026-10-08 17:36 ` Darrick J. Wong
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2026-10-08 12:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, stable, linux-xfs
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 12/12] xfs: complain loudly when dirtree checker finds broken links
2026-10-08 12:04 ` Christoph Hellwig
@ 2026-10-08 17:36 ` Darrick J. Wong
0 siblings, 0 replies; 30+ messages in thread
From: Darrick J. Wong @ 2026-10-08 17:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: cem, stable, linux-xfs
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
^ permalink raw reply [flat|nested] 30+ messages in thread