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

On Wed, Jul 15, 2026 at 11:07:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <[email protected]>
> 
> 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?