| Message ID | 20241125084111.141386-1-allison.karlitskaya@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | btrfs: add FS_IOC_READ_VERITY_METADATA ioctl | expand |
On Mon, Nov 25, 2024 at 09:41:11AM +0100, Allison Karlitskaya wrote: > e17fe6579de0 introduced FS_IOC_READ_VERITY_METADATA to directly query > the Merkle tree, descriptor and signature blocks for fs-verity enabled > files. It also added the ioctl implementation to ext4 and f2fs, but > seems to have forgotten about btrfs. At the time, btrfs did not support fs-verity. > > Add the (trival) implementation for btrfs: we just need to wire it > through to the fs-verity code, the same way as was done for the other > two filesystems. The fs-verity code already has access to the required > data. This ioctl isn't too useful, but I suppose adding it to btrfs can't hurt. Can you run xfstests generic/624 and generic/625, which test this ioctl? - Eric
hi Eric, Thanks for the reply. On Mon, 25 Nov 2024 at 19:11, Eric Biggers <ebiggers@kernel.org> wrote: > At the time, btrfs did not support fs-verity. Oops. I missed that detail. :) I wonder why they did the implementation without the metadata ioctl, then... Would you like me to change the commit message? (or feel free to do it yourself...) > This ioctl isn't too useful, but I suppose adding it to btrfs can't hurt. I ran into the missing implementation because I'm trying to use it here: https://github.com/tytso/e2fsprogs/pull/203 for the ultimate purpose of this: https://github.com/containers/composefs-rs/blob/main/examples/uki/build tl;dr: I'm trying to build filesystem images in user-space with fs-verity-enabled files inside of them, by copying the metadata up from the host filesystem. I have btrfs on my work machine, so for me this ioctl is definitely very useful at the moment. :) > Can you run xfstests generic/624 and generic/625, which test this ioctl? I managed to get a patched Fedora kernel built (which was quite an ordeal) and ran the requested tests. Here's a session log: root@fedora:/home/lis/xfstests# cat local.config # Ideally define at least these 4 to match your environment # The first 2 are required. # See README for other variables which can be set. # # Note: SCRATCH_DEV >will< get overwritten! export TEST_DEV=/dev/loop0 export TEST_DIR=/mnt/test export SCRATCH_DEV=/dev/loop1 export SCRATCH_MNT=/mnt/scratch export FSTYP=btrfs root@fedora:/home/lis/xfstests# ./check generic/624 generic/625 FSTYP -- btrfs PLATFORM -- Linux/x86_64 fedora 6.11.10-300.fc41.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Nov 25 12:51:20 UTC 2024 MKFS_OPTIONS -- /dev/loop1 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/loop1 /mnt/scratch generic/624 1s ... 1s generic/625 [not run] kernel doesn't support fs-verity builtin signatures Ran: generic/624 generic/625 Not run: generic/625 Passed all 2 tests root@fedora:/home/lis/xfstests# dd if=/dev/urandom of=4096 bs=4096 count=1 1+0 records in 1+0 records out 4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000132844 s, 30.8 MB/s root@fedora:/home/lis/xfstests# dd if=/dev/urandom of=4097 bs=4097 count=1 1+0 records in 1+0 records out 4097 bytes (4.1 kB, 4.0 KiB) copied, 0.000104737 s, 39.1 MB/s root@fedora:/home/lis/xfstests# fsverity enable 4096 root@fedora:/home/lis/xfstests# fsverity enable 4097 root@fedora:/home/lis/xfstests# fsverity dump_metadata merkle_tree 4096 | hexdump -C root@fedora:/home/lis/xfstests# fsverity dump_metadata descriptor 4096 | hexdump -C 00000000 01 01 0c 00 00 00 00 00 00 10 00 00 00 00 00 00 |................| 00000010 8c 23 a1 d2 cf 86 16 40 2c 60 cd 04 05 a8 03 db |.#.....@,`......| 00000020 8e f7 1e 4e c8 bc 9b 78 ce 9c dd 1d a2 22 44 e0 |...N...x....."D.| 00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00000100 root@fedora:/home/lis/xfstests# fsverity dump_metadata merkle_tree 4097 | hexdump -C 00000000 96 b4 01 30 4c 9a 13 2b b0 97 da 5d d2 5f 19 e4 |...0L..+...]._..| 00000010 ff 34 d9 70 40 80 68 99 82 db 14 5f b6 00 f9 cd |.4.p@.h...._....| 00000020 33 16 3f 33 a0 2f c6 e7 74 f8 65 b6 d9 e0 af 5f |3.?3./..t.e...._| 00000030 53 2b 11 7f d9 02 57 e1 89 b8 ea e7 d6 26 26 dc |S+....W......&&.| 00000040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00001000 root@fedora:/home/lis/xfstests# fsverity dump_metadata descriptor 4097 | hexdump -C 00000000 01 01 0c 00 00 00 00 00 01 10 00 00 00 00 00 00 |................| 00000010 7a fb db 76 d0 17 54 62 4c 17 28 b9 d1 19 a6 fe |z..v..TbL.(.....| 00000020 56 56 5e d9 e3 90 f9 f5 02 74 a3 f4 65 4f ea a5 |VV^......t..eO..| 00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00000100 root@fedora:/home/lis/xfstests# As you can see, the Fedora kernel doesn't have signature support, so that test got skipped. Indeed, the config contains this: # CONFIG_FS_VERITY_BUILTIN_SIGNATURES is not set # CONFIG_FS_VERITY_DEBUG is not set CONFIG_FS_VERITY=y I guess it's not particularly relevant to verity the functioning of this API, though. In its place, I've included some manual tests for querying the merkle_tree (both for a file that gets directly hashed into the descriptor, and also for one that requires a tree layer) plus the descriptors for those. I'd really prefer if I didn't have to build another kernel: my laptop isn't so beefy and this one already took most of my work day... Please let me know if you need any extra information. Thanks again, lis
On Mon, Nov 25, 2024 at 09:06:25PM +0100, Allison Karlitskaya wrote: > hi Eric, > > Thanks for the reply. > > On Mon, 25 Nov 2024 at 19:11, Eric Biggers <ebiggers@kernel.org> wrote: > > At the time, btrfs did not support fs-verity. > > Oops. I missed that detail. :) I wonder why they did the > implementation without the metadata ioctl, then... > > Would you like me to change the commit message? (or feel free to do > it yourself...) Please go ahead and update it. Thanks! BTW, I recommend that this be taken through the btrfs tree. > > This ioctl isn't too useful, but I suppose adding it to btrfs can't hurt. > > I ran into the missing implementation because I'm trying to use it here: > https://github.com/tytso/e2fsprogs/pull/203 > for the ultimate purpose of this: > https://github.com/containers/composefs-rs/blob/main/examples/uki/build > > tl;dr: I'm trying to build filesystem images in user-space with > fs-verity-enabled files inside of them, by copying the metadata up > from the host filesystem. I have btrfs on my work machine, so for me > this ioctl is definitely very useful at the moment. :) Hmm, interesting. I guess that makes sense, though this wasn't an anticipated use case for this ioctl. Maybe the documentation for FS_IOC_READ_VERITY_METADATA in Documentation/filesystems/fsverity.rst could use an update to mention this use case. > I guess it's not particularly relevant to verity the functioning of > this API, though. > > In its place, I've included some manual tests for querying the > merkle_tree (both for a file that gets directly hashed into the > descriptor, and also for one that requires a tree layer) plus the > descriptors for those. I'd really prefer if I didn't have to build > another kernel: my laptop isn't so beefy and this one already took > most of my work day... > > Please let me know if you need any extra information. Thanks for testing it! It should be enough for now, but in the future for kernel patches I'm afraid you need to get used to building kernels. - Eric
On Tue, Nov 26, 2024 at 02:33:13AM +0000, Eric Biggers wrote: > On Mon, Nov 25, 2024 at 09:06:25PM +0100, Allison Karlitskaya wrote: > > hi Eric, > > > > Thanks for the reply. > > > > On Mon, 25 Nov 2024 at 19:11, Eric Biggers <ebiggers@kernel.org> wrote: > > > At the time, btrfs did not support fs-verity. > > > > Oops. I missed that detail. :) I wonder why they did the > > implementation without the metadata ioctl, then... > > > > Would you like me to change the commit message? (or feel free to do > > it yourself...) > > Please go ahead and update it. Thanks! > > BTW, I recommend that this be taken through the btrfs tree. Yes this should go through our tree. Thanks for the patch and evaluation. I've noticed one patch in fstests being skipped due to the missing ioctl. Nobody knows why it wasn't added or what it could be useful for but as there is a use case then we'll add it.
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index c9302d193187..392322a70ce8 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -5290,6 +5290,8 @@ long btrfs_ioctl(struct file *file, unsigned int return fsverity_ioctl_enable(file, (const void __user *)argp); case FS_IOC_MEASURE_VERITY: return fsverity_ioctl_measure(file, argp); + case FS_IOC_READ_VERITY_METADATA: + return fsverity_ioctl_read_metadata(file, argp); case BTRFS_IOC_ENCODED_READ: return btrfs_ioctl_encoded_read(file, argp, false); case BTRFS_IOC_ENCODED_WRITE:
e17fe6579de0 introduced FS_IOC_READ_VERITY_METADATA to directly query the Merkle tree, descriptor and signature blocks for fs-verity enabled files. It also added the ioctl implementation to ext4 and f2fs, but seems to have forgotten about btrfs. Add the (trival) implementation for btrfs: we just need to wire it through to the fs-verity code, the same way as was done for the other two filesystems. The fs-verity code already has access to the required data. FS_IOC_READ_VERITY_METADATA remains unimplemented for FUSE, but implementing it there would be more involved. Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com> --- fs/btrfs/ioctl.c | 2 ++ 1 file changed, 2 insertions(+)