[PATCH] xfs: detect cycles in recovered unlinked inode lists

[Michael Bommarito]

Two XFS walkers follow on-disk AGI unlinked bucket lists until the
NULLAGINO sentinel and trust the links unconditionally:

  xlog_recover_iunlink_bucket(), reached at mount time when the log
  needs recovery, via xlog_recover_finish() ->
  xlog_recover_process_iunlinks().

  xfs_inode_reload_unlinked_bucket(), reached post-mount via
  xfs_bulkstat_one_int() and other inode-reload paths.

A crafted image with an AGI bucket head pointing into a cycle traps
either walker forever. With the mount-time walker, the mount(8)
syscall itself never returns; the kernel thread is uninterruptible
inside the loop, so SIGKILL on the mount process is queued but never
delivered. With the bulkstat walker, an XFS_IOC_BULKSTAT call hangs
the same way.

Reject invalid AG inode numbers, bucket mismatches, and repeated AG
inode numbers in both walkers. Mark the AGI sick and return
-EFSCORRUPTED so callers handle the corrupt metadata instead of
walking the same on-disk list forever.

Reproduced on a crafted image whose AGI bucket holds a self-cycle and
whose on-disk log is dirty: a stock kernel hangs in the mount syscall
indefinitely, while the patched kernel completes log recovery,
reports the corruption, and the filesystem shuts down with
-EFSCORRUPTED.

Fixes: 04755d2e5821b3afbaadd09fe5df58d04de36484 ("xfs: refactor xlog_recover_process_iunlinks()")
Fixes: 83771c50e42b92de6740a63e152c96c052d37736 ("xfs: reload entire unlinked bucket lists")
Assisted-by: Codex:gpt-5-5-xhigh
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
---
 fs/xfs/xfs_inode.c       | 29 +++++++++++++++++++++++++++++
 fs/xfs/xfs_log_recover.c | 26 +++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index beaa26ec62da4..f930d5c823c77 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -4,6 +4,7 @@
  * All Rights Reserved.
  */
 #include <linux/iversion.h>
+#include <linux/xarray.h>
 
 #include "xfs_platform.h"
 #include "xfs_fs.h"
@@ -2859,6 +2860,7 @@ xfs_inode_reload_unlinked_bucket(
 	struct xfs_buf		*agibp;
 	struct xfs_agi		*agi;
 	struct xfs_perag	*pag;
+	struct xarray		seen_aginos;
 	xfs_agnumber_t		agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
 	xfs_agino_t		agino = XFS_INO_TO_AGINO(mp, ip->i_ino);
 	xfs_agino_t		prev_agino, next_agino;
@@ -2873,6 +2875,8 @@ xfs_inode_reload_unlinked_bucket(
 	if (error)
 		return error;
 
+	xa_init(&seen_aginos);
+
 	/*
 	 * We've taken ILOCK_SHARED and the AGI buffer lock to stabilize the
 	 * incore unlinked list pointers for this inode.  Check once more to
@@ -2897,6 +2901,30 @@ xfs_inode_reload_unlinked_bucket(
 	while (next_agino != NULLAGINO) {
 		struct xfs_inode	*next_ip = NULL;
 
+		/*
+		 * The on-disk unlinked list is corrupt if it points outside this
+		 * AG, into another bucket, or back to an inode that we already
+		 * saw during this reload walk.
+		 */
+		if (!xfs_verify_agino(pag, next_agino) ||
+		    next_agino % XFS_AGI_UNLINKED_BUCKETS != bucket) {
+			xfs_buf_mark_corrupt(agibp);
+			xfs_ag_mark_sick(pag, XFS_SICK_AG_AGI);
+			error = -EFSCORRUPTED;
+			break;
+		}
+
+		if (xa_load(&seen_aginos, next_agino)) {
+			xfs_buf_mark_corrupt(agibp);
+			xfs_ag_mark_sick(pag, XFS_SICK_AG_AGI);
+			error = -EFSCORRUPTED;
+			break;
+		}
+		error = xa_err(xa_store(&seen_aginos, next_agino,
+					xa_mk_value(1), GFP_NOFS));
+		if (error)
+			break;
+
 		/* Found this caller's inode, set its backlink. */
 		if (next_agino == agino) {
 			next_ip = ip;
@@ -2931,6 +2959,7 @@ xfs_inode_reload_unlinked_bucket(
 	}
 
 out_agibp:
+	xa_destroy(&seen_aginos);
 	xfs_trans_brelse(tp, agibp);
 	/* Should have found this inode somewhere in the iunlinked bucket. */
 	if (!error && !foundit)
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 09e6678ca4878..8ca70bbbfac81 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3,6 +3,8 @@
  * Copyright (c) 2000-2006 Silicon Graphics, Inc.
  * All Rights Reserved.
  */
+#include <linux/xarray.h>
+
 #include "xfs_platform.h"
 #include "xfs_fs.h"
 #include "xfs_shared.h"
@@ -28,6 +30,7 @@
 #include "xfs_ag.h"
 #include "xfs_quota.h"
 #include "xfs_reflink.h"
+#include "xfs_health.h"
 
 #define BLK_AVG(blk1, blk2)	((blk1+blk2) >> 1)
 
@@ -2726,11 +2729,31 @@ xlog_recover_iunlink_bucket(
 	struct xfs_mount	*mp = pag_mount(pag);
 	struct xfs_inode	*prev_ip = NULL;
 	struct xfs_inode	*ip;
+	struct xarray		seen_aginos;
 	xfs_agino_t		prev_agino, agino;
 	int			error = 0;
 
+	xa_init(&seen_aginos);
+
 	agino = be32_to_cpu(agi->agi_unlinked[bucket]);
 	while (agino != NULLAGINO) {
+		if (!xfs_verify_agino(pag, agino) ||
+		    agino % XFS_AGI_UNLINKED_BUCKETS != bucket) {
+			xfs_ag_mark_sick(pag, XFS_SICK_AG_AGI);
+			error = -EFSCORRUPTED;
+			break;
+		}
+
+		if (xa_load(&seen_aginos, agino)) {
+			xfs_ag_mark_sick(pag, XFS_SICK_AG_AGI);
+			error = -EFSCORRUPTED;
+			break;
+		}
+		error = xa_err(xa_store(&seen_aginos, agino, xa_mk_value(1),
+					GFP_NOFS));
+		if (error)
+			break;
+
 		error = xfs_iget(mp, NULL, xfs_agino_to_ino(pag, agino), 0, 0,
 				&ip);
 		if (error)
@@ -2771,8 +2794,9 @@ xlog_recover_iunlink_bucket(
 
 		error2 = xfs_inodegc_flush(mp);
 		if (error2 && !error)
-			return error2;
+			error = error2;
 	}
+	xa_destroy(&seen_aginos);
 	return error;
 }
 
-- 
2.53.0