[PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[Allison Karlitskaya]

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(+)

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:
-- 
2.47.0

Re: [PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[Eric Biggers]

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

Re: [PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[Allison Karlitskaya]

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

Re: [PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[Eric Biggers]

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

Re: [PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[David Sterba]

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.

[PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[Allison Karlitskaya]

146054090b085 introduced fs-verity support for btrfs, but didn't add
support for FS_IOC_READ_VERITY_METADATA to directly query the Merkle
tree, descriptor and signature blocks for fs-verity enabled files.

Add the (trival) implementation: we just need to wire it through to the
fs-verity code, the same way as is done in the other two filesystems
which support this ioctl (ext4, f2fs). The fs-verity code already has
access to the required data.

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
---
 fs/btrfs/ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

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:
-- 
2.47.0

Re: [PATCH] btrfs: add FS_IOC_READ_VERITY_METADATA ioctl

[David Sterba]

On Tue, Nov 26, 2024 at 04:23:31PM +0100, Allison Karlitskaya wrote:
> 146054090b085 introduced fs-verity support for btrfs, but didn't add

Please use full reference of commits with the subject lines, like
146054090b08 ("btrfs: initial fsverity support")

The git alias command for that is below, you can name it like you want:

	show -s --pretty='format:%h (\"%s\")'

> support for FS_IOC_READ_VERITY_METADATA to directly query the Merkle
> tree, descriptor and signature blocks for fs-verity enabled files.
> 
> Add the (trival) implementation: we just need to wire it through to the
> fs-verity code, the same way as is done in the other two filesystems
> which support this ioctl (ext4, f2fs). The fs-verity code already has
> access to the required data.
> 
> Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>

Added to for-next, thanks.