xfs: abort directory parent scrub scans if we encounter a zapped directory
authorDarrick J. Wong <djwong@kernel.org>
Fri, 15 Dec 2023 18:03:37 +0000 (10:03 -0800)
committerDarrick J. Wong <djwong@kernel.org>
Fri, 15 Dec 2023 18:03:37 +0000 (10:03 -0800)
In a previous patch, we added some code to perform sufficient repairs
to an ondisk inode record such that the inode cache would be willing to
load the inode.  If the broken inode was a shortform directory, it will
reset the directory to something plausible, which is to say an empty
subdirectory of the root.  The telltale signs that something is
seriously wrong is the broken link count.

Such directories look clean, but they shouldn't participate in a
filesystem scan to find or confirm a directory parent pointer.  Create a
predicate that identifies such directories and abort the scrub.

Found by fuzzing xfs/1554 with multithreaded xfs_scrub enabled and
u3.bmx[0].startblock = zeroes.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
fs/xfs/scrub/common.c
fs/xfs/scrub/common.h
fs/xfs/scrub/dir.c
fs/xfs/scrub/parent.c

index f0207e71e5dc6f3044425a9d184a1025c4d2c97d..81f2b96bb5a74256e4bdfb9ff9e7fb951a30633a 100644 (file)
@@ -25,6 +25,7 @@
 #include "xfs_trans_priv.h"
 #include "xfs_da_format.h"
 #include "xfs_da_btree.h"
+#include "xfs_dir2_priv.h"
 #include "xfs_attr.h"
 #include "xfs_reflink.h"
 #include "xfs_ag.h"
index c69cacb0b69620e86879a589bab1dd03131adc3e..ec5755266259d33901efcabede8f705bf9fe27cb 100644 (file)
@@ -198,6 +198,8 @@ static inline bool xchk_skip_xref(struct xfs_scrub_metadata *sm)
                               XFS_SCRUB_OFLAG_XCORRUPT);
 }
 
+bool xchk_dir_looks_zapped(struct xfs_inode *dp);
+
 #ifdef CONFIG_XFS_ONLINE_REPAIR
 /* Decide if a repair is required. */
 static inline bool xchk_needs_repair(const struct xfs_scrub_metadata *sm)
index b366fab699ac505e8ceea96b62816020b56587b2..d86ab51af9282dac1b7f83ffde194a3c2acf54ba 100644 (file)
@@ -798,3 +798,29 @@ xchk_directory(
        xchk_mark_healthy_if_clean(sc, XFS_SICK_INO_DIR_ZAPPED);
        return 0;
 }
+
+/*
+ * Decide if this directory has been zapped to satisfy the inode and ifork
+ * verifiers.  Checking and repairing should be postponed until the directory
+ * is fixed.
+ */
+bool
+xchk_dir_looks_zapped(
+       struct xfs_inode        *dp)
+{
+       /* Repair zapped this dir's data fork a short time ago */
+       if (xfs_ifork_zapped(dp, XFS_DATA_FORK))
+               return true;
+
+       /*
+        * If the dinode repair found a bad data fork, it will reset the fork
+        * to extents format with zero records and wait for the bmapbtd
+        * scrubber to reconstruct the block mappings.  Directories always
+        * contain some content, so this is a clear sign of a zapped directory.
+        * The state checked by xfs_ifork_zapped is not persisted, so this is
+        * the secondary strategy if repairs are interrupted by a crash or an
+        * unmount.
+        */
+       return dp->i_df.if_format == XFS_DINODE_FMT_EXTENTS &&
+              dp->i_df.if_nextents == 0;
+}
index e6155d86f7916d367fff4742867dcdf110bff05e..7db8736721461c5d643a7a67a09fbb69b7abfaff 100644 (file)
@@ -156,6 +156,16 @@ xchk_parent_validate(
                goto out_rele;
        }
 
+       /*
+        * We cannot yet validate this parent pointer if the directory looks as
+        * though it has been zapped by the inode record repair code.
+        */
+       if (xchk_dir_looks_zapped(dp)) {
+               error = -EBUSY;
+               xchk_set_incomplete(sc);
+               goto out_unlock;
+       }
+
        /* Look for a directory entry in the parent pointing to the child. */
        error = xchk_dir_walk(sc, dp, xchk_parent_actor, &spc);
        if (!xchk_fblock_xref_process_error(sc, XFS_DATA_FORK, 0, &error))
@@ -217,6 +227,13 @@ xchk_parent(
                 */
                error = xchk_parent_validate(sc, parent_ino);
        } while (error == -EAGAIN);
+       if (error == -EBUSY) {
+               /*
+                * We could not scan a directory, so we marked the check
+                * incomplete.  No further error return is necessary.
+                */
+               return 0;
+       }
 
        return error;
 }