[PATCH 5.10] fsverity: Explicitly require FMODE_READ to retrieve hashes

[PATCH 5.10] fsverity: Explicitly require FMODE_READ to retrieve hashes

[Eric Biggers]

commit 1acb4fd2d011c4bd6cc961095ae17361c71ff61a upstream.

The comment in fsverity_ioctl_read_metadata(), which justifies not
checking for FMODE_READ, doesn't consider that the nonstandard access
mode 3 (ioctl-only) exists.

Opening an ioctl-only fd generally requires read (and write) permission.
However, there are gaps with certain LSMs: an ioctl-only fd requests
read (and write) permission via security_inode_permission, but not
security_file_open.  With certain LSMs relying on the latter, such as
Landlock and AppArmor, an ioctl-only fd can be opened without the LSM
granting read (and write) permission.

Since FS_IOC_READ_VERITY_METADATA returns hashes of file contents
blocks, it should require read permission.  This also applies to
FS_IOC_MEASURE_VERITY, which returns a hash for the whole file.

Meanwhile, ioctl-only fds are a rarely-used feature, not known to be
used with either ioctl.

Therefore, just disallow these ioctls on ioctl-only file descriptors,
similar to what was already done with FS_IOC_ENABLE_VERITY.

More specifically, explicitly check for FMODE_READ.  This also covers
write-only fds to non-verity files which otherwise would have failed
with ENODATA.  This change in error code for write-only fds should be
fine too, as these ioctls cannot be usefully used with write-only fds
anyway, but it's worth noting as another technicality.

Fixes: 4dd893d832cf ("fs-verity: implement FS_IOC_MEASURE_VERITY ioctl")
Fixes: e17fe6579de0 ("fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl")
Cc: stable@vger.kernel.org
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
Link: https://patch.msgid.link/20261022211522.1489943-1-ebiggers@kernel.org
(5.10: dropped the FS_IOC_READ_VERITY_METADATA changes, since that ioctl
 does not exist in this version)
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 Documentation/filesystems/fsverity.rst |  2 ++
 fs/verity/measure.c                    | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 895e9711ed881..4bd6f25e1a4b3 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -146,6 +146,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
@@ -207,6 +208,7 @@ regardless of the size of the file.
 
 FS_IOC_MEASURE_VERITY can fail with the following errors:
 
+- ``EBADF``: the file descriptor is not open for reading
 - ``EFAULT``: the caller provided inaccessible memory
 - ``ENODATA``: the file is not a verity file
 - ``ENOTTY``: this type of filesystem does not implement fs-verity
diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index df409a5682edf..be3628f9e557f 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -28,6 +28,16 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
 	const struct fsverity_hash_alg *hash_alg;
 	struct fsverity_digest arg;
 
+	/*
+	 * This ioctl returns a hash of the file contents, so it should require
+	 * read permission.  Disallow it on ioctl-only (i.e., nonstandard access
+	 * mode 3) fds, which can in some cases be acquired without passing all
+	 * LSM read-permission checks.  Also disallow write-only fds, which can
+	 * refer only to non-verity files which would fail with ENODATA anyway.
+	 */
+	if (!(filp->f_mode & FMODE_READ))
+		return -EBADF;
+
 	vi = fsverity_get_info(inode);
 	if (!vi)
 		return -ENODATA; /* not a verity file */

base-commit: 3a323bc22b881a5bdc1646080f93f5ecb22850bf
-- 
2.56.0

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

[sashiko-bot]

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

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

fsverity: Explicitly require FMODE_READ to retrieve hashes

This commit backports the FMODE_READ requirement for FS_IOC_MEASURE_VERITY
and updates the fs-verity documentation for EBADF error returns.

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

> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 895e9711ed881..4bd6f25e1a4b3 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -146,6 +146,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

[Severity: Medium]
Does fsverity_ioctl_enable() actually check FMODE_READ or return -EBADF in
5.10?

Upstream commit 04839139213c ("fsverity: reject FS_IOC_ENABLE_VERITY on
mode 3 fds"), which added the FMODE_READ check to fsverity_ioctl_enable(),
has not been backported to 5.10:

fs/verity/enable.c:fsverity_ioctl_enable() {
	...
	err = file_permission(filp, MAY_WRITE);
	if (err)
		return err;
	...
	err = deny_write_access(filp);
	if (err) /* -ETXTBSY */
		goto out_drop_write;

	err = enable_verity(filp, &arg);
	...
}

Without that commit, FS_IOC_ENABLE_VERITY fails with -ETXTBSY rather than
-EBADF on writable file descriptors, and still allows ioctl-only (access
mode 3) file descriptors to proceed without FMODE_READ.  Consequently, an
ioctl-only fd opened without LSM read/write checks in security_file_open()
can still read the file contents to build the Merkle tree and enable
verity on the file.

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