xfs: fix another iunlink infinite loop bug in online fsck

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 remembering which inodes we've already seen and
checking new agino pointers against that.  If a bit is already set,
either this is a loop or the inode has nonzero link count.  We'll deal
with the second case in a subsequent patch.

Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 41de5cf87352b4383c262597cccf75b62739f90a..65b9a8befce99be4a0a00b16b84892475f5519c6 100644 (file)
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -1348,15 +1348,32 @@ xrep_iunlink_resolve_bucket(
        struct xrep_agi         *ragi,
        unsigned int            bucket)
 {
+       struct xagino_bitmap    seen;
        struct xfs_scrub        *sc = ragi->sc;
        struct xfs_inode        *ip;
        xfs_agino_t             prev_agino = NULLAGINO;
        xfs_agino_t             next_agino = ragi->iunlink_heads[bucket];
        int                     error = 0;
 
+       xagino_bitmap_init(&seen);
+
        while (next_agino != NULLAGINO) {
+               unsigned int len = 1;
+
                if (xchk_should_terminate(ragi->sc, &error))
-                       return error;
+                       goto out_bitmap;
+
+               /* Inode already seen?  We're stuck in a loop */
+               if (xagino_bitmap_test(&seen, next_agino, &len)) {
+                       trace_xrep_iunlink_resolve_infinite_loop(sc->sa.pag,
+                                       bucket, prev_agino, next_agino);
+                       next_agino = NULLAGINO;
+                       break;
+               }
+
+               error = xagino_bitmap_set(&seen, next_agino, 1);
+               if (error)
+                       goto out_bitmap;
 
                /* Find the next inode in the chain. */
                ip = xfs_iunlink_lookup(sc->sa.pag, next_agino);
@@ -1382,17 +1399,17 @@ xrep_iunlink_resolve_bucket(
                        error = xrep_iunlink_store_next(ragi, next_agino,
                                        NULLAGINO);
                        if (error)
-                               return error;
+                               goto out_bitmap;
 
                        error = xrep_iunlink_store_prev(ragi, next_agino,
                                        LINKED_AGINO);
                        if (error)
-                               return error;
+                               goto out_bitmap;
 
                        error = xagino_bitmap_clear(&ragi->iunlink_bmp,
                                        next_agino, 1);
                        if (error)
-                               return error;
+                               goto out_bitmap;
 
                        next_agino = ip->i_next_unlinked;
                        continue;
@@ -1433,20 +1450,20 @@ xrep_iunlink_resolve_bucket(
                 */
                error = xagino_bitmap_clear(&ragi->iunlink_bmp, next_agino, 1);
                if (error)
-                       return error;
+                       goto out_bitmap;
 
                /* Remember the previous inode's next pointer. */
                if (prev_agino != NULLAGINO) {
                        error = xrep_iunlink_store_next(ragi, prev_agino,
                                        next_agino);
                        if (error)
-                               return error;
+                               goto out_bitmap;
                }
 
                /* Remember this inode's previous pointer. */
                error = xrep_iunlink_store_prev(ragi, next_agino, prev_agino);
                if (error)
-                       return error;
+                       goto out_bitmap;
 
                /* Advance the list and remember this inode. */
                prev_agino = next_agino;
@@ -1457,10 +1474,12 @@ xrep_iunlink_resolve_bucket(
        if (prev_agino != NULLAGINO) {
                error = xrep_iunlink_store_next(ragi, prev_agino, next_agino);
                if (error)
-                       return error;
+                       goto out_bitmap;
        }
 
-       return 0;
+out_bitmap:
+       xagino_bitmap_destroy(&seen);
+       return error;
 }
 
 /* Reinsert this unlinked inode into the head of the staged bucket list. */

diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index 00fbe1b9c2354fe1302c218d56daf0e01ea82187..14aa0ec1f09e4ab70d2cee3b0e50ca475dbac447 100644 (file)
--- 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);