Re: [PATCH 6.1] fsverity: Explicitly require FMODE_READ to retrieve hashes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] fsverity: missing FMODE_READ check in fsverity_ioctl_enable()
--

commit ce00f17646315fc0b3de9e3b97325887e3d4a262
Author: Eric Biggers <ebiggers@kernel.org>

fsverity: Explicitly require FMODE_READ to retrieve hashes

This commit backports upstream commit 1acb4fd2d011c4bd6cc961095ae17361c71ff61a to require FMODE_READ in fsverity_ioctl_measure() and fsverity_ioctl_read_metadata(), and documents that FS_IOC_ENABLE_VERITY, FS_IOC_MEASURE_VERITY, and FS_IOC_READ_VERITY_METADATA fail with EBADF when the file descriptor is not open for reading.

Link: https://patch.msgid.link/20261022211522.1489943-1-ebiggers@kernel.org

> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index cb8e7573882a1..3c7ddd6790baa 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -157,6 +157,7 @@ fatal signal), no changes are made to the file.
>  FS_IOC_ENABLE_VERITY can fail with the following errors:
>  
>  - ``EACCES``: the process does not have write access to the file
> +- ``EBADF``: the file descriptor is not open for reading
>  - ``EBADMSG``: the signature is malformed
>  - ``EBUSY``: this ioctl is already running on the file
>  - ``EEXIST``: the file already has verity enabled

[Severity: Medium]
Does fsverity_ioctl_enable() also need an explicit FMODE_READ check in this
branch?

In mainline, fsverity_ioctl_enable() gained a check for FMODE_READ when
Merkle tree construction was converted to use __kernel_read().  That
change is not in this tree, where fsverity_ioctl_enable() only checks
file_permission(filp, MAY_WRITE) and deny_write_access(filp), and
build_merkle_tree_level() reads directly from the page cache via
read_file_data_page().

As a result, FS_IOC_ENABLE_VERITY never returns -EBADF when the file
descriptor is not open for reading (returning -ETXTBSY on O_WRONLY fds and
succeeding on mode 3 ioctl-only fds).  A process confined by Landlock or
AppArmor can still open a DAC-writable file with access mode 3 without
passing security_file_open() read/write checks and invoke
FS_IOC_ENABLE_VERITY to hash the file contents and permanently enable
fs-verity.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107191050.25921-1-ebiggers@kernel.org?part=1