[PATCH 1/8] xfs_scrub: fix missing scrub coverage for broken inodes

Darrick J. Wong
2023-12-31T22:38:32Z

If INUMBERS says that an inode is allocated, but BULKSTAT skips over the
inode and BULKSTAT_SINGLE errors out when loading the inumber, there are
two possibilities: One, we're racing with ifree; or two, the inode is
corrupt and iget failed.

When this happens, the scrub_scan_all_inodes code will insert a dummy
bulkstat record with all fields zeroed except bs_ino and bs_blksize.
Hence the use of i_mode switches in phase3 to schedule file content
scrubbing are not entirely correct -- bs_mode==0 means "type unknown",
which ought to mean "schedule all scrubbers".

Unfortunately, the current code doesn't do that, so instead we schedule
no content scrubs. If the broken file was actually a directory, we fail
to check the directory contents for further corruptions.

Found by using fuzzing with xfs/385 and core.format = 0.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
scrub/phase3.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/scrub/phase3.c b/scrub/phase3.c
index 9a26b92036c..b03b55250a3 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -166,16 +166,29 @@ scrub_inode(
if (error)
goto out;

- if (S_ISLNK(bstat->bs_mode)) {
+ /*
+ * Check file data contents, e.g. symlink and directory entries.
+ *
+ * Note: bs_mode==0 occurs when inumbers says an inode is allocated,
+ * bulkstat skips the inode, and bulkstat_single errors out when
+ * loading the inode. This could be due to racing with ifree, but it
+ * could be a corrupt inode. Either way, schedule all the data fork
+ * content scrubbers. Better to have them return -ENOENT than miss
+ * some coverage.
+ */
+ if (S_ISLNK(bstat->bs_mode) || !bstat->bs_mode) {
/* Check symlink contents. */
error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_SYMLINK,
&alist);
- } else if (S_ISDIR(bstat->bs_mode)) {
+ if (error)
+ goto out;
+ }
+ if (S_ISDIR(bstat->bs_mode) || !bstat->bs_mode) {
/* Check the directory entries. */
error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_DIR, &alist);
+ if (error)
+ goto out;
}
- if (error)
- goto out;

/* Check all the extended attributes. */
error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_XATTR, &alist);