Re: [PATCH 8/9] xfs: fix another iunlink infinite loop bug in online fsck

On Mon, Jul 20, 2026 at 08:26:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <[email protected]>
> 
> xrep_iunlink_resolve_bucket is supposed to reconstruct as much of the
> incore prev and next unlinked list pointers based on what it finds on
> disk and in memory before we move on to relinking the truly lost inodes
> back into the unlinked list.  However, it's still vulnerable to infinite
> loops that come in via the next_unlinked pointers.
> 
> Fix this problem by checking iunlink_bmp, which at that point in
> execution represents truly unlinked inodes that haven't yet been
> processed.  If a bit is already unset, either this is a loop or the
> inode has nonzero link count.  We'll deal with the second case in a
> subsequent patch.
> 
> Cc: <[email protected]> # v6.10
> Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
> Signed-off-by: "Darrick J. Wong" <[email protected]>
> ---
>  fs/xfs/scrub/trace.h           |    1 +
>  fs/xfs/scrub/agheader_repair.c |   10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 00fbe1b9c2354f..14aa0ec1f09e4a 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -3538,6 +3538,7 @@ DEFINE_EVENT(xrep_iunlink_resolve_class, name, \
>  	TP_PROTO(const struct xfs_perag *pag, unsigned int bucket, \
>  		 xfs_agino_t prev_agino, xfs_agino_t next_agino), \
>  	TP_ARGS(pag, bucket, prev_agino, next_agino))
> +DEFINE_REPAIR_IUNLINK_RESOLVE_EVENT(xrep_iunlink_resolve_infinite_loop);
>  DEFINE_REPAIR_IUNLINK_RESOLVE_EVENT(xrep_iunlink_resolve_uncached);
>  DEFINE_REPAIR_IUNLINK_RESOLVE_EVENT(xrep_iunlink_resolve_wronglist);
>  DEFINE_REPAIR_IUNLINK_RESOLVE_EVENT(xrep_iunlink_resolve_nolist);
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index edd406a97b7ce9..da2265678d17f5 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -1355,6 +1355,8 @@ xrep_iunlink_resolve_bucket(
>  	int			error = 0;
>  
>  	while (next_agino != NULLAGINO) {
> +		unsigned int len = 1;
> +
>  		if (xchk_should_terminate(ragi->sc, &error))
>  			return error;
>  
> @@ -1417,6 +1419,14 @@ xrep_iunlink_resolve_bucket(
>  			continue;
>  		}
>  
> +		/* Inode already seen?  We're stuck in a loop */
> +		if (!xagino_bitmap_test(&ragi->iunlink_bmp, next_agino, &len)) {

Hmm.  Having given this patch a little more thought, I think
xrep_iunlink_resolve_bucket should have its own "seen inode" bitmap
for inodes that it has seen in the loop body rather than trying to reuse
iunlink_bmp for this purpose.

iunlink_bmp is created as the bitmap of inodes that have zero nlink,
but once we start fixing the iunlink bucket chains, it becomes the
bitmap of inodes with zero nlink that also aren't reachable in the
ondisk metadata.  The inodes that remain set in iunlink_bmp are the ones
that have to be put back on the list.

Loop detection, OTOH, just needs to know what we've already seen whilst
walking the chian.  That could be nonzero nlink inodes due to corruption
problems or whatever.  So let's make the code easier to understand by
using a separate bitmap like the agheader.c iunlink verifier does.

I'll go fix this patch tomorrow and repost.

--D

> +			trace_xrep_iunlink_resolve_infinite_loop(sc->sa.pag,
> +					bucket, prev_agino, next_agino);
> +			next_agino = NULLAGINO;
> +			break;
> +		}
> +
>  		trace_xrep_iunlink_resolve_ok(sc->sa.pag, bucket, prev_agino,
>  				next_agino);
>  
> 
>