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. 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