xfs: don't livelock in scrub on a circular unlinked list

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
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index cecf034ef989c7a5623f9d2fad5b030d78d3e0fb..1fa66aa68e169fb4a73d21b7e09a6ff75ccac576 100644 (file)
--- 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(
@@ -935,7 +937,8 @@ xchk_agi_xref(
 /*
  * Walk the incore unlinked list for a particular AGI bucket to construct
  * the unlinked inode bitmap for later reconstruction of the unlinked list.
- * Returns 1 if we should keep checking, or 0 to stop checking.
+ * Returns 1 if we should keep checking, 0 to stop checking, or a negative
+ * errno.
  */
 static int
 xchk_iunlink_bucket(
@@ -943,36 +946,57 @@ xchk_iunlink_bucket(
        unsigned int                    bucket,
        xfs_agino_t                     agino)
 {
+       struct xagino_bitmap            seen;
+       int                             ret;
+
+       xagino_bitmap_init(&seen);
+
        while (agino != NULLAGINO) {
                struct xfs_inode        *ip;
+               unsigned int            len = 1;
 
                if (agino % XFS_AGI_UNLINKED_BUCKETS != bucket) {
                        xchk_block_set_corrupt(sc, sc->sa.agi_bp);
-                       return 0;
+                       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);
-                       return 0;
+                       goto bad;
                }
 
                if (!xfs_inode_on_unlinked_list(ip)) {
                        xchk_block_set_corrupt(sc, sc->sa.agi_bp);
-                       return 0;
+                       goto bad;
                }
 
+               ret = xagino_bitmap_set(&seen, agino, 1);
+               if (ret)
+                       goto out_bitmap;
+
                agino = ip->i_next_unlinked;
        }
-
-       return 1;
+       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)
@@ -985,8 +1009,10 @@ xchk_iunlink(
                ret = xchk_iunlink_bucket(sc, i,
                                be32_to_cpu(agi->agi_unlinked[i]));
                if (ret < 1)
-                       return;
+                       return ret;
        }
+
+       return 0;
 }
 
 /* Scrub the AGI. */
@@ -1073,7 +1099,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 2554494847ff1bdfdd333862742cda29a61818fb..13074d5e319cc0a9eb1615227eed922245d2a53a 100644 (file)
--- 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. */