Re: [PATCH 5/7] xfs: abort directory parent scrub scans if we encounter a zapped directory

Darrick J. Wong
2023-11-30T21:37:10Z

On Wed, Nov 29, 2023 at 08:47:34PM -0800, Christoph Hellwig wrote:
> On Fri, Nov 24, 2023 at 03:52:23PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > In the 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.
>
> This is now a few commits back. My adjust this to be less specific.
>
> > 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.
>
> This kind of ties into my comment on the previous comment, but needing
> heuristics to find zapped inodes or inode forks just seems to be asking
> for trouble. I suspect we'll need proper on-disk flags to notice the
> corrupted / half-rebuilt state.

Hmm. A single "zapped" bit would be a good way to signal to
xchk_dir_looks_zapped and xchk_bmap_want_check_rmaps that a file is
probably broken. Clearing that bit would be harder though -- userspace
would have to call back into the kernel after checking all the metadata.

A simpler way might be to persist the entire per-inode sick state (both
forks and the contents within, for three bits). That would be more to
track, but each scrubber could clear its corresponding sick-state bit.
A bit further on in this series is a big patchset to set the sick state
every time the hot paths encounter an EFSCORRUPTED.

IO operations could check the sick state bit and fail out to userspace,
which would solve the problem of keeping programs away from a partially
fixed file.

The ondisk state tracking like an entire project on its own. Thoughts?

--D