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

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

[Darrick J. Wong]

Hi all,

Here's a third 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-3
---
Commits in this patchset:
 * xfs: check cowextsize in xrep_inode_cowextsize
 * xfs: fix transaction block reservation in xrep_rtbitmap
 * xfs: rtsummary scrub should treat rtbitmap corruption errors as an xref error
 * xfs: zero i_nlink before repair puts inode on unlinked list
 * xfs: mark nonzero sb_gquotino as corrupt on metadir filesystems
 * xfs: don't livelock in scrub on a circular unlinked list
---
 fs/xfs/scrub/agheader.c        |   90 +++++++++++++++++++++++++++++-----------
 fs/xfs/scrub/agheader_repair.c |   17 +++++++-
 fs/xfs/scrub/inode_repair.c    |    2 -
 fs/xfs/scrub/nlinks_repair.c   |    7 +++
 fs/xfs/scrub/rtbitmap_repair.c |   46 ++++++++++++--------
 fs/xfs/scrub/rtsummary.c       |    2 -
 6 files changed, 117 insertions(+), 47 deletions(-)

[PATCH 1/6] xfs: check cowextsize in xrep_inode_cowextsize

[Darrick J. Wong]

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

LOLLM points out that the function that corrects cowextsize should check
i_cowextsize, not i_extsize.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: a9600db96f74af ("xfs: detect and repair misaligned rtinherit directory cowextsize hints")
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 3ec41c19835116..b88427a4460cba 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1960,7 +1960,7 @@ xrep_inode_cowextsize(
 	/* Fix misaligned CoW extent size hints on a directory. */
 	if ((sc->ip->i_diflags & XFS_DIFLAG_RTINHERIT) &&
 	    (sc->ip->i_diflags2 & XFS_DIFLAG2_COWEXTSIZE) &&
-	    sc->ip->i_extsize % sc->mp->m_sb.sb_rextsize > 0) {
+	    xfs_extlen_to_rtxmod(sc->mp, sc->ip->i_cowextsize) > 0) {
 		sc->ip->i_cowextsize = 0;
 		sc->ip->i_diflags2 &= ~XFS_DIFLAG2_COWEXTSIZE;
 	}

[PATCH 2/6] xfs: fix transaction block reservation in xrep_rtbitmap

[Darrick J. Wong]

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

LOLLM pointed out an inconsistency in the block reservation code in
xrep_rtbitmap.  The first is that the reservation computation is not
consistent between the code that sets up the repair and the code that
tries to avoid exceeding the transaction reservation once we know how
big the rtbitmap really must be.  As a result, the logic doesn't work.

In fixing that, a second problem emerges: if we do readjust, we ask for
the entire reservation all over again.  We really only need the delta,
so ask only for that.

Fix all these problems by hoisting the computation to a trivial helper
so that it gets used in both places.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 8defee8dff2b20 ("xfs: online repair of realtime bitmaps for a realtime group")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/rtbitmap_repair.c |   46 ++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 18 deletions(-)


diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index dc64902d6c25a4..442a17bf972029 100644
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -36,6 +36,24 @@
 
 /* rt bitmap content repairs */
 
+/*
+ * Reserve enough blocks to write out a completely new bitmap file, plus twice
+ * as many blocks as we would need if we can only allocate one block per data
+ * fork mapping.  This should cover the preallocation of the temporary file and
+ * exchanging the extent mappings.
+ *
+ * We cannot use xfs_exchmaps_estimate because we have not yet constructed the
+ * replacement bitmap and therefore do not know how many extents it will use.
+ * By the time we do, we will have a dirty transaction (which we cannot drop
+ * because we cannot drop the rtbitmap ILOCK) and cannot ask for more
+ * reservation.
+ */
+static inline unsigned long long
+xrep_rtbitmap_calc_blocks(struct xfs_mount *mp, unsigned long long blocks)
+{
+	return blocks + (xfs_bmbt_calc_size(mp, blocks) * 2);
+}
+
 /* Set up to repair the realtime bitmap for this group. */
 int
 xrep_setup_rtbitmap(
@@ -56,20 +74,7 @@ xrep_setup_rtbitmap(
 	if (error)
 		return error;
 
-	/*
-	 * Reserve enough blocks to write out a completely new bitmap file,
-	 * plus twice as many blocks as we would need if we can only allocate
-	 * one block per data fork mapping.  This should cover the
-	 * preallocation of the temporary file and exchanging the extent
-	 * mappings.
-	 *
-	 * We cannot use xfs_exchmaps_estimate because we have not yet
-	 * constructed the replacement bitmap and therefore do not know how
-	 * many extents it will use.  By the time we do, we will have a dirty
-	 * transaction (which we cannot drop because we cannot drop the
-	 * rtbitmap ILOCK) and cannot ask for more reservation.
-	 */
-	blocks += xfs_bmbt_calc_size(mp, blocks) * 2;
+	blocks = xrep_rtbitmap_calc_blocks(mp, mp->m_sb.sb_rbmblocks);
 	if (blocks > UINT_MAX)
 		return -EOPNOTSUPP;
 
@@ -512,7 +517,7 @@ xrep_rtbitmap(
 	struct xchk_rtbitmap	*rtb = sc->buf;
 	struct xfs_mount	*mp = sc->mp;
 	struct xfs_group	*xg = rtg_group(sc->sr.rtg);
-	unsigned long long	blocks = 0;
+	unsigned long long	blocks;
 	unsigned int		busy_gen;
 	int			error;
 
@@ -532,15 +537,20 @@ xrep_rtbitmap(
 	 * figure out if we need to adjust the block reservation in the
 	 * transaction.
 	 */
-	blocks = xfs_bmbt_calc_size(mp, rtb->rbmblocks);
+	blocks = xrep_rtbitmap_calc_blocks(mp, rtb->rbmblocks);
 	if (blocks > UINT_MAX)
 		return -EOPNOTSUPP;
 	if (blocks > rtb->resblks) {
-		error = xfs_trans_reserve_more(sc->tp, blocks, 0);
+		uint64_t	delta = blocks - rtb->resblks;
+
+		if (delta > UINT_MAX)
+			return -EOPNOTSUPP;
+
+		error = xfs_trans_reserve_more(sc->tp, delta, 0);
 		if (error)
 			return error;
 
-		rtb->resblks += blocks;
+		rtb->resblks += delta;
 	}
 
 	/* Fix inode core and forks. */

[PATCH 3/6] xfs: rtsummary scrub should treat rtbitmap corruption errors as an xref error

[Darrick J. Wong]

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

LOLLM notes the discrepancy between the comment saying that this is an
xref error and the code, which marks a (non-xref) corruption error.
This appears to be a regression.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: e3088ae2dcae3c ("xfs: move RT bitmap and summary information to the rtgroup")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/rtsummary.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
index 78f72a0468870e..546b335ade1354 100644
--- a/fs/xfs/scrub/rtsummary.c
+++ b/fs/xfs/scrub/rtsummary.c
@@ -358,7 +358,7 @@ xchk_rtsummary(
 		 * EFSCORRUPTED means the rtbitmap is corrupt, which is an xref
 		 * error since we're checking the summary file.
 		 */
-		xchk_ip_set_corrupt(sc, rbmip);
+		xchk_ip_xref_set_corrupt(sc, rbmip);
 		return 0;
 	}
 	if (error)

[PATCH 4/6] xfs: zero i_nlink before repair puts inode on unlinked list

[Darrick J. Wong]

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

LOLLM observes that we don't reset i_nlink if we encounter a file with
no parent that isn't on the unlinked list.  This causes unnecessary
assertion trips on debugging kernels and an inconsistent file, so let's
fix that.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 669dfe883c8e20 ("xfs: update the unlinked list when repairing link counts")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/nlinks_repair.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/nlinks_repair.c b/fs/xfs/scrub/nlinks_repair.c
index fbc2ff809fc06f..09e097e1668935 100644
--- a/fs/xfs/scrub/nlinks_repair.c
+++ b/fs/xfs/scrub/nlinks_repair.c
@@ -232,9 +232,14 @@ xrep_nlinks_repair_inode(
 	 * unlinked list, put it on the unlinked list.
 	 */
 	if (total_links == 0 && !xfs_inode_on_unlinked_list(ip)) {
+		if (actual_nlink)
+			clear_nlink(VFS_I(ip));
 		error = xfs_iunlink(sc->tp, ip);
-		if (error)
+		if (error) {
+			if (actual_nlink)
+				set_nlink(VFS_I(ip), actual_nlink);
 			goto out_trans;
+		}
 		dirty = true;
 	}
 

[PATCH 5/6] xfs: mark nonzero sb_gquotino as corrupt on metadir filesystems

[Darrick J. Wong]

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

On a metadir filesystem, the superblock gquotino field is always zero
because we find the quota inodes through the metadata directory tree.
A nonzero value is therefore a corruption (as pointed out by LOLLM) so
mark the failure as such.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: 06b20ef09ba163 ("xfs: check pre-metadir fields correctly")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/agheader.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 9ed053b5f06125..62ed5eaf08fbc1 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -266,7 +266,7 @@ xchk_superblock(
 			xchk_block_set_corrupt(sc, bp);
 
 		if (sb->sb_gquotino != cpu_to_be64(0))
-			xchk_block_set_preen(sc, bp);
+			xchk_block_set_corrupt(sc, bp);
 	} else {
 		if (sb->sb_uquotino != cpu_to_be64(mp->m_sb.sb_uquotino))
 			xchk_block_set_preen(sc, bp);

[PATCH 6/6] xfs: don't livelock in scrub on a circular unlinked list

[Darrick J. Wong]

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

LOLLM points out that online fsck can livelock if an unlinked inode list
contains a loop.  Use a bitmap to detect cycles.

Cc: <stable@vger.kernel.org> # v4.15
Fixes: a12890aebb8959 ("xfs: scrub the AGI")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/agheader.c        |   88 ++++++++++++++++++++++++++++++----------
 fs/xfs/scrub/agheader_repair.c |   17 +++++++-
 2 files changed, 80 insertions(+), 25 deletions(-)


diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 62ed5eaf08fbc1..0ecf969f95a9d2 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -18,6 +18,8 @@
 #include "xfs_inode.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
+#include "scrub/bitmap.h"
+#include "scrub/agino_bitmap.h"
 
 int
 xchk_setup_agheader(
@@ -932,41 +934,79 @@ xchk_agi_xref(
 	/* scrub teardown will take care of sc->sa for us */
 }
 
+static int
+xchk_iunlink_bucket(
+	struct xfs_scrub	*sc,
+	unsigned int		bucket,
+	xfs_agino_t		agino)
+{
+	struct xagino_bitmap	seen;
+	struct xfs_inode	*ip;
+	int			ret = 0;
+
+	xagino_bitmap_init(&seen);
+
+	while (agino != NULLAGINO) {
+		unsigned int	len = 1;
+
+		if (agino % XFS_AGI_UNLINKED_BUCKETS != bucket) {
+			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
+			goto bad;
+		}
+
+		if (xagino_bitmap_test(&seen, agino, &len)) {
+			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
+			goto bad;
+		}
+
+		ip = xfs_iunlink_lookup(sc->sa.pag, agino);
+		if (!ip) {
+			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
+			goto bad;
+		}
+
+		if (!xfs_inode_on_unlinked_list(ip)) {
+			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
+			goto bad;
+		}
+
+		ret = xagino_bitmap_set(&seen, agino, 1);
+		if (ret)
+			goto out_bitmap;
+
+		agino = ip->i_next_unlinked;
+	}
+	ret = 1;
+
+out_bitmap:
+	xagino_bitmap_destroy(&seen);
+	return ret;
+bad:
+	ret = 0;
+	goto out_bitmap;
+}
+
 /*
  * Check the unlinked buckets for links to bad inodes.  We hold the AGI, so
  * there cannot be any threads updating unlinked list pointers in this AG.
  */
-STATIC void
+STATIC int
 xchk_iunlink(
 	struct xfs_scrub	*sc,
 	struct xfs_agi		*agi)
 {
 	unsigned int		i;
-	struct xfs_inode	*ip;
 
 	for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) {
-		xfs_agino_t	agino = be32_to_cpu(agi->agi_unlinked[i]);
+		int		ret;
 
-		while (agino != NULLAGINO) {
-			if (agino % XFS_AGI_UNLINKED_BUCKETS != i) {
-				xchk_block_set_corrupt(sc, sc->sa.agi_bp);
-				return;
-			}
-
-			ip = xfs_iunlink_lookup(sc->sa.pag, agino);
-			if (!ip) {
-				xchk_block_set_corrupt(sc, sc->sa.agi_bp);
-				return;
-			}
-
-			if (!xfs_inode_on_unlinked_list(ip)) {
-				xchk_block_set_corrupt(sc, sc->sa.agi_bp);
-				return;
-			}
-
-			agino = ip->i_next_unlinked;
-		}
+		ret = xchk_iunlink_bucket(sc, i,
+					be32_to_cpu(agi->agi_unlinked[i]));
+		if (ret < 1)
+			return ret;
 	}
+
+	return 0;
 }
 
 /* Scrub the AGI. */
@@ -1053,7 +1093,9 @@ xchk_agi(
 	if (pag->pagi_freecount != be32_to_cpu(agi->agi_freecount))
 		xchk_block_set_corrupt(sc, sc->sa.agi_bp);
 
-	xchk_iunlink(sc, agi);
+	error = xchk_iunlink(sc, agi);
+	if (error)
+		goto out;
 
 	xchk_agi_xref(sc);
 out:
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 2554494847ff1b..13074d5e319cc0 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -1080,18 +1080,22 @@ xrep_iunlink_walk_ondisk_bucket(
 	struct xrep_agi		*ragi,
 	unsigned int		bucket)
 {
+	struct xagino_bitmap	seen;
 	struct xfs_scrub	*sc = ragi->sc;
 	struct xfs_agi		*agi = sc->sa.agi_bp->b_addr;
 	xfs_agino_t		prev_agino = NULLAGINO;
 	xfs_agino_t		next_agino;
 	int			error = 0;
 
+	xagino_bitmap_init(&seen);
+
 	next_agino = be32_to_cpu(agi->agi_unlinked[bucket]);
 	while (next_agino != NULLAGINO) {
 		xfs_agino_t	agino = next_agino;
+		unsigned int	len = 1;
 
 		if (xchk_should_terminate(ragi->sc, &error))
-			return error;
+			goto out_bitmap;
 
 		trace_xrep_iunlink_walk_ondisk_bucket(sc->sa.pag, bucket,
 				prev_agino, agino);
@@ -1099,15 +1103,24 @@ xrep_iunlink_walk_ondisk_bucket(
 		if (bucket != agino % XFS_AGI_UNLINKED_BUCKETS)
 			break;
 
+		if (xagino_bitmap_test(&seen, agino, &len))
+			break;
+
 		next_agino = xrep_iunlink_next(sc, agino);
 		if (!next_agino)
 			next_agino = xrep_iunlink_reload_next(ragi, prev_agino,
 					agino);
 
+		error = xagino_bitmap_set(&seen, agino, 1);
+		if (error)
+			goto out_bitmap;
+
 		prev_agino = agino;
 	}
 
-	return 0;
+out_bitmap:
+	xagino_bitmap_destroy(&seen);
+	return error;
 }
 
 /* Decide if this is an unlinked inode in this AG. */

Re: [PATCH 1/6] xfs: check cowextsize in xrep_inode_cowextsize

[Christoph Hellwig]

On Wed, Jul 15, 2026 at 11:06:22PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that the function that corrects cowextsize should check
> i_cowextsize, not i_extsize.

Looks good:

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

Re: [PATCH 2/6] xfs: fix transaction block reservation in xrep_rtbitmap

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 3/6] xfs: rtsummary scrub should treat rtbitmap corruption errors as an xref error

[Christoph Hellwig]

On Wed, Jul 15, 2026 at 11:06:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM notes the discrepancy between the comment saying that this is an
> xref error and the code, which marks a (non-xref) corruption error.
> This appears to be a regression.

Looks good:

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

Re: [PATCH 4/6] xfs: zero i_nlink before repair puts inode on unlinked list

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 5/6] xfs: mark nonzero sb_gquotino as corrupt on metadir filesystems

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 6/6] xfs: don't livelock in scrub on a circular unlinked list

[Christoph Hellwig]

On Wed, Jul 15, 2026 at 11:07:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that online fsck can livelock if an unlinked inode list
> contains a loop.  Use a bitmap to detect cycles.

Didn't we recently have some AI generated patch for the same thing?

Either way, this does actually look reasonable.  But also complex
enough that I really want a test case that creates an image with
such a corruption to test this case before we merge the kernel code
as this is something that basically requires a maliciously crafted
image, and we trade new otherwise untested code for a theoretical
bug.

> +static int
> +xchk_iunlink_bucket(
> +	struct xfs_scrub	*sc,
> +	unsigned int		bucket,
> +	xfs_agino_t		agino)

Maybe add a comment what this function tests?

> +{
> +	struct xagino_bitmap	seen;
> +	struct xfs_inode	*ip;
> +	int			ret = 0;
> +
> +	xagino_bitmap_init(&seen);
> +
> +	while (agino != NULLAGINO) {
> +		unsigned int	len = 1;
> +
> +		if (agino % XFS_AGI_UNLINKED_BUCKETS != bucket) {
> +			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
> +			goto bad;
> +		}

Handle entries that should not be here, makes sense.

> +
> +		if (xagino_bitmap_test(&seen, agino, &len)) {
> +			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
> +			goto bad;
> +		}

Check that we don't have duplicates, makes sense.

> +		ip = xfs_iunlink_lookup(sc->sa.pag, agino);
> +		if (!ip) {
> +			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
> +			goto bad;
> +		}
> +
> +		if (!xfs_inode_on_unlinked_list(ip)) {
> +			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
> +			goto bad;
> +		}

The that that the inode actually is on the unlinked list, makes
sense.

>  	for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) {
> -		xfs_agino_t	agino = be32_to_cpu(agi->agi_unlinked[i]);
> +		int		ret;
>  
> -		while (agino != NULLAGINO) {
> -			if (agino % XFS_AGI_UNLINKED_BUCKETS != i) {
> -				xchk_block_set_corrupt(sc, sc->sa.agi_bp);
> -				return;
> -			}

Ahh, and this is mostly existing code...

Maybe split the reactoring into a separate helper into a prep patch?