Re: [PATCHSET v5.3] fs-verity support for XFS

["Darrick J. Wong" <djwong@kernel.org>]

[fix tinguely email addr]

On Tue, Mar 19, 2024 at 03:07:43PM -0700, Darrick J. Wong wrote:
> On Mon, Mar 18, 2024 at 09:35:12AM -0700, Eric Biggers wrote:
> > On Sun, Mar 17, 2024 at 09:22:52AM -0700, Darrick J. Wong wrote:
> > > Hi all,
> > > 
> > > From Darrick J. Wong:
> > > 
> > > This v5.3 patchset builds upon v5.2 of Andrey's patchset to implement
> > > fsverity for XFS.
> > 
> > Is this ready for me to review, or is my feedback on v5 still being
> > worked on?
> 
> It's still being worked on.  I figured it was time to push my work tree
> back to Andrey so everyone could see the results of me attempting to
> understand the fsverity patchset by working around in the codebase.
> 
> From your perspective, I suspect the most interesting patches will be 5,
> 6, 7+10+14, 11-13, and 15-17.  For everyone on the XFS side, patches
> 27-39 are the most interesting since they change the caching strategy
> and slim down the ondisk format.
> 
> > From a quick glance, not everything from my feedback has been
> > addressed.
> 
> That's correct.  I cleaned up the mechanics of passing merkle trees
> around, but I didn't address the comments about per-sb workqueues,
> fsverity tracepoints, or whether or not iomap should allocate biosets.

That perhaps wasn't quite clear enough -- I'm curious to see what Andrey
has to say about that part (patches 8, 9, 18) of the patchset.

--D

> Roughly, here's what I did in the generic code:
> 
> I fixed the FS_XFLAG_VERITY handling so that you can't clear it via
> FS_IOC_FSSETXATTR.
> 
> I also rewrote and augmented the "drop dead merkle tree" functions in
> xfs_verity to clean out incomplete trees when ->end_enable tells us we
> failed; and to clean out extra blocks in the ->begin_enable just in case
> the file shrank since a failed attempt to enable fsverity.
> 
> As for online repair, the "fsverity: expose merkle tree geometry to
> callers" enables the kernel to do some basic online checking that there
> aren't excessive merkle tree blocks and that fsverity can read the
> descriptor.  In my djwong-wtf tree, xfs_scrub gains the ability to read
> the entire file into the pagecache (and hence validate the verity info)
> via MADV_POPULATE READ, and now it has a patch to read the entire merkle
> tree/descriptor/signature just to make sure those can actually be read.
> 
> Most of the things you gave feedback about in "fsverity: support
> block-based Merkle tree caching" I think I cleaned up in "fsverity: fix
> "support block-based Merkle tree caching"" and "fsverity: rely on cached
> block callers to retain verified state".  I kept those separate so that
> Andrey could see what I did, though they really ought to be merged into
> the main support patch.
> 
> Note that I greatly expanded the usage of struct fsverity_blockbuf and
> changed the verified flag handling so that the invalidation function was
> no longer necessary.
> 
> --D
> 
> > - Eric
>