> On Feb 5, 2021, at 1:58 AM, Boris Burkov <bo...@bur.io> wrote: > > On Thu, Feb 04, 2021 at 10:13:54PM -0800, Eric Biggers wrote: >> On Thu, Feb 04, 2021 at 03:21:36PM -0800, Boris Burkov wrote: >>> This patchset provides support for fsverity in btrfs. >> >> Very interested to see this! It generally looks good, but I have some >> comments. >> >> Also, when you send this out next, can you include >> linux-fscr...@vger.kernel.org, as per 'get_maintainer.pl fs/verity/'? >> > > Sorry for missing that, definitely will do for v2. > >>> At a high level, we store the verity descriptor and Merkle tree data >>> in the file system btree with the file's inode as the objectid, and >>> direct reads/writes to those items to implement the generic fsverity >>> interface required by fs/verity/. >>> >>> The first patch is a preparatory patch which adds a notion of >>> compat_flags to the btrfs_inode and inode_item in order to allow >>> enabling verity on a file without making the file system unmountable for >>> older kernels. (It runs afoul of the leaf corruption check otherwise) >> >> In ext4, verity is a ro_compat filesystem feature rather than a compat >> feature. >> That's because we wanted to prevent old kernels from writing to verity files, >> which would corrupt them (get them out of sync with their Merkle trees). >> >> Are you sure you want to make this a "compat" flag? >> > > I wasn't sure, so I'm glad you brought it up. That's a pretty compelling > argument for making it ro_comnpat, in my opinion. I was also worried > about the old kernel deleting the file and leaking the Merkle items. >
Deleting the file will also delete the verity items, on both old and new kernels. btrfs_truncate_inode_items() doesn’t mess around. > On the other hand, it feels a shame to make the whole file system read > only over "just one file". > I don’t feel really strongly, but lean towards read/write for this reason. Being consistent with other implementations is important though. > Do you have any good strategies for getting back a file system after > creating some verity files but then running a kernel without verity? > > I could write some utilities to list/delete verity files before doing > that transition? > >>> >>> The second patch is the bulk of the fsverity implementation. It >>> implements the fsverity interface and adds verity checks for the typical >>> file reading case. >>> >>> The third patch cleans up the corner cases in readpage, covering inline >>> extents, preallocated extents, and holes. >>> >>> The fourth patch handles direct io of a veritied file by falling back to >>> buffered io. >>> >>> The fifth patch adds a feature file in sysfs for verity. >> >> I'm also wondering if you've tested using this in combination with btrfs >> compression. f2fs also supports compression and verity in combination, and >> there have been some problems caused by that combination not being properly >> tested. It should just work though. >> > > I hadn't tested it with compression yet, but I'll definitely do so, > especially since it was a pain point before. Thanks for the tip. I did test these in the initial implementation, but more is always better. The verity is on the uncompressed pages, so the verity pass happens after btrfs crcs and btrfs compression. -chris
