From 1acb4fd2d011c4bd6cc961095ae17361c71ff61a Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 22 Oct 2026 21:15:22 +0000 Subject: [PATCH 1/4] fsverity: Explicitly require FMODE_READ to retrieve hashes 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 Link: https://patch.msgid.link/20261022211522.1489943-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- Documentation/filesystems/fsverity.rst | 3 +++ fs/verity/measure.c | 10 ++++++++++ fs/verity/read_metadata.c | 14 ++++++++++---- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst index 22b49b295d1f..e5d38439bf58 100644 --- a/Documentation/filesystems/fsverity.rst +++ b/Documentation/filesystems/fsverity.rst @@ -170,6 +170,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 builtin signature is malformed - ``EBUSY``: this ioctl is already running on the file - ``EEXIST``: the file already has verity enabled @@ -232,6 +233,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 @@ -310,6 +312,7 @@ blocks from disk without actually verifying the path to the root node. FS_IOC_READ_VERITY_METADATA can fail with the following errors: +- ``EBADF``: the file descriptor is not open for reading - ``EFAULT``: the caller provided inaccessible memory - ``EINTR``: the ioctl was interrupted before any data was read - ``EINVAL``: reserved fields were set, or ``offset + length`` diff --git a/fs/verity/measure.c b/fs/verity/measure.c index 68dfccb69772..0c75616dca01 100644 --- a/fs/verity/measure.c +++ b/fs/verity/measure.c @@ -31,6 +31,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 */ diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c index b4c0892430cd..6694149f7b46 100644 --- a/fs/verity/read_metadata.c +++ b/fs/verity/read_metadata.c @@ -166,13 +166,19 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg) int length; void __user *buf; + /* + * This ioctl returns file contents block hashes, 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 */ - /* - * Note that we don't have to explicitly check that the file is open for - * reading, since verity files can only be opened for reading. - */ if (copy_from_user(&arg, uarg, sizeof(arg))) return -EFAULT; From 8f8faec82aa902b1997ae578c46bcc84afbad811 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 25 Oct 2026 13:38:31 -0700 Subject: [PATCH 2/4] fsverity: Be more careful with return value of ->get_verity_descriptor() If fsverity_operations::get_verity_descriptor() returns that the verity descriptor has 0 size, then a zero-size buffer is allocated to hold it, and it's called again with bufsize=0. But bufsize=0 is a special case that says to return the actual size. If in the meantime the filesystem has decided that the size is some actually valid size, it will return that size, and then validate_fsverity_descriptor() will read from ZERO_SIZE_PTR and crash. Of course, for the filesystem to misbehave in that strange way, it generally would require direct writes to the block device. Regardless, fsverity_get_descriptor() might as well guard against it. Make it do so. Specifically, validate that the first call to ->get_verity_descriptor() returns at least the minimum descriptor size, and validate that the second call returns the same size as the first. Fixes: fd2d1acfcadf ("fs-verity: add the hook for file ->open()") Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20261025203831.120095-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/verity/open.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/fs/verity/open.c b/fs/verity/open.c index 664629043038..78a65154b895 100644 --- a/fs/verity/open.c +++ b/fs/verity/open.c @@ -270,12 +270,6 @@ static bool validate_fsverity_descriptor(struct inode *inode, const struct fsverity_descriptor *desc, size_t desc_size) { - if (desc_size < sizeof(*desc)) { - fsverity_err(inode, "Unrecognized descriptor size: %zu bytes", - desc_size); - return false; - } - if (desc->version != 1) { fsverity_err(inode, "Unrecognized descriptor version: %u", desc->version); @@ -314,31 +308,37 @@ static bool validate_fsverity_descriptor(struct inode *inode, int fsverity_get_descriptor(struct inode *inode, struct fsverity_descriptor **desc_ret) { - int res; + int size, res; struct fsverity_descriptor *desc; - res = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0); - if (res < 0) { - fsverity_err(inode, - "Error %d getting verity descriptor size", res); - return res; + size = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0); + if (size < 0) { + fsverity_err(inode, "Error %d getting verity descriptor size", + size); + return size; } - if (res > FS_VERITY_MAX_DESCRIPTOR_SIZE) { - fsverity_err(inode, "Verity descriptor is too large (%d bytes)", - res); + if (size < sizeof(*desc) || size > FS_VERITY_MAX_DESCRIPTOR_SIZE) { + fsverity_err(inode, + "Verity descriptor has invalid size (%d bytes)", + size); return -EMSGSIZE; } - desc = kmalloc(res, GFP_KERNEL); + desc = kmalloc(size, GFP_KERNEL); if (!desc) return -ENOMEM; - res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, res); + res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, size); if (res < 0) { fsverity_err(inode, "Error %d reading verity descriptor", res); kfree(desc); return res; } - - if (!validate_fsverity_descriptor(inode, desc, res)) { + if (res != size) { + fsverity_err(inode, "Verity descriptor size changed (%d => %d)", + size, res); + kfree(desc); + return -EINVAL; + } + if (!validate_fsverity_descriptor(inode, desc, size)) { kfree(desc); return -EINVAL; } From 28d74059fddbd8ae83c7715056b303c402071f6f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 25 Oct 2026 13:45:34 -0700 Subject: [PATCH 3/4] fsverity: Tell LSMs about signature only after verity is enabled FS_IOC_ENABLE_VERITY calls security_inode_setintegrity() before verity has finished being enabled. That's broken because enabling verity can still fail at that point, e.g. due to insufficient xattr space. Afterwards, arbitrary contents can be written to the file, which doesn't actually have verity enabled. The in-core inode stays the same, so the LSM considers it to have verity enabled with a valid signature. (Or rather, in the case of IPE, which is the only LSM that implements security_inode_setintegrity(), it also checks IS_VERITY(), which can be bypassed by enabling verity with no signature after the file writes.) Fix this by moving the call to security_inode_setintegrity() out of fsverity_verify_signature() and into a new function that is called from the appropriate places. Fixes: 7c373e4f1445 ("fsverity: expose verified fsverity built-in signatures to LSMs") Cc: stable@vger.kernel.org Reviewed-by: Fan Wu Tested-by: Fan Wu Link: https://patch.msgid.link/20261025204534.122506-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/verity/enable.c | 8 ++++++ fs/verity/fsverity_private.h | 8 ++++++ fs/verity/open.c | 9 ++++++ fs/verity/signature.c | 55 ++++++++++++++++++++++++++---------- 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/fs/verity/enable.c b/fs/verity/enable.c index ad4ff71d7dd9..2949fb8afb2f 100644 --- a/fs/verity/enable.c +++ b/fs/verity/enable.c @@ -302,6 +302,14 @@ static int enable_verity(struct file *filp, } else if (WARN_ON_ONCE(!IS_VERITY(inode))) { fsverity_remove_info(vi); err = -EINVAL; + } else { + /* + * Now that verity is enabled, report the signature (if any) to + * LSMs. We tolerate that a process racing with this ioctl can + * briefly see verity enabled with no signature reported yet. + */ + fsverity_report_valid_sig(inode, desc->signature, + le32_to_cpu(desc->sig_size)); } out: kfree(params.hashstate); diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h index 6f85f63861aa..2c41428d484b 100644 --- a/fs/verity/fsverity_private.h +++ b/fs/verity/fsverity_private.h @@ -160,6 +160,8 @@ void __init fsverity_init_info_cache(void); extern int fsverity_require_signatures; int fsverity_verify_signature(const struct fsverity_info *vi, const u8 *signature, size_t sig_size); +void fsverity_report_valid_sig(const struct inode *inode, const u8 *signature, + size_t sig_size); void __init fsverity_init_signature(void); #else /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */ @@ -170,6 +172,12 @@ fsverity_verify_signature(const struct fsverity_info *vi, return 0; } +static inline void fsverity_report_valid_sig(const struct inode *inode, + const u8 *signature, + size_t sig_size) +{ +} + static inline void fsverity_init_signature(void) { } diff --git a/fs/verity/open.c b/fs/verity/open.c index 78a65154b895..d1ac9556e947 100644 --- a/fs/verity/open.c +++ b/fs/verity/open.c @@ -387,6 +387,15 @@ int fsverity_ensure_verity_info(struct inode *inode) goto out_free_desc; } + /* + * Now that the signature (if any) has been verified, report it to LSMs. + * Do this before inserting the fsverity_info into the hashtable, so + * that anyone who concurrently retrieves the fsverity_info for the same + * inode is guaranteed to see the LSM's signature status populated. + */ + fsverity_report_valid_sig(inode, desc->signature, + le32_to_cpu(desc->sig_size)); + /* * Multiple tasks may race to set the inode's verity info, in which case * we might find an existing fsverity_info in the hash table. diff --git a/fs/verity/signature.c b/fs/verity/signature.c index 07657e913b22..2f49c5d9d987 100644 --- a/fs/verity/signature.c +++ b/fs/verity/signature.c @@ -42,13 +42,16 @@ static struct key *fsverity_keyring; * @sig_size: size of signature in bytes, or 0 if no signature * * If the file includes a signature of its fs-verity file digest, verify it - * against the certificates in the fs-verity keyring. Note that signatures - * are verified regardless of the state of the 'fsverity_require_signatures' - * variable and the LSM subsystem relies on this behavior to help enforce - * file integrity policies. Please discuss changes with the LSM list - * (thank you!). + * against the certificates in the fs-verity keyring. * - * Return: 0 on success (signature valid or not required); -errno on failure + * Note that signatures are verified regardless of the state of the + * 'fsverity_require_signatures' variable, and the LSM subsystem relies on this + * behavior to help enforce file integrity policies. (See + * fsverity_report_valid_sig(), which assumes that the signature, if present, + * was verified here.) Please discuss changes with the LSM list (thank you!). + * + * Return: 0 if the signature is present and valid, or absent and not required; + * -errno on failure */ int fsverity_verify_signature(const struct fsverity_info *vi, const u8 *signature, size_t sig_size) @@ -111,18 +114,40 @@ int fsverity_verify_signature(const struct fsverity_info *vi, return err; } - err = security_inode_setintegrity(inode, - LSM_INT_FSVERITY_BUILTINSIG_VALID, - signature, - sig_size); + return 0; +} - if (err) { +/* + * If the signature exists, this tells the LSM that it was verified. + * + * fsverity_verify_signature() must have already been called with the same + * signature, *and* the inode must have verity enabled with the signature + * matching it. If called from FS_IOC_ENABLE_VERITY, the latter means that we + * must be past the point of no return, i.e. verity won't fail to be enabled at + * a later point. This is critical, since LSMs must be told about the signature + * only when the verity status is permanent in the in-core inode. + */ +void fsverity_report_valid_sig(const struct inode *inode, const u8 *signature, + size_t sig_size) +{ + int err; + + if (WARN_ON_ONCE(!IS_VERITY(inode))) + return; + + if (sig_size == 0) + return; + + err = security_inode_setintegrity( + inode, LSM_INT_FSVERITY_BUILTINSIG_VALID, signature, sig_size); + if (err) fsverity_err(inode, "Error %d exposing file signature to LSMs", err); - return err; - } - - return 0; + /* + * Besides the logging above, ignore errors here. The LSM hook just + * records the signature and doesn't perform access control. And in the + * case of FS_IOC_ENABLE_VERITY verity was already enabled. + */ } void __init fsverity_init_signature(void) From c349b233d6ab0fc3517401a1e2d36f5f90eb8c9b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 25 Oct 2026 13:48:07 -0700 Subject: [PATCH 4/4] fsverity: Set S_VERITY only after rollback is no longer possible Commit f77f281b6118 ("fsverity: use a hashtable to find the fsverity_info") changed the implementation of FS_IOC_ENABLE_VERITY to make concurrent reads start going through fsverity verification slightly earlier than they did before. Instead of it happening when enable_verity() calls fsverity_set_info(), it now happens when ->end_enable_verity() sets S_VERITY. However, ext4 and btrfs can roll back verity after they set S_VERITY, causing a use-after-free. Fix that by making ext4 and btrfs set S_VERITY only after rollback is no longer possible. On failure, don't roll back S_VERITY, but do roll back the filesystem-specific verity flag. Also update enable_verity() to handle any future instances of this bug more safely by warning and *not* freeing the fsverity_info. Finally, tweak some comments. Clarify one, and move information from another to its proper place in the comment for ->end_enable_verity(). Fixes: f77f281b6118 ("fsverity: use a hashtable to find the fsverity_info") Cc: stable@vger.kernel.org Reviewed-by: Christoph Hellwig Link: https://patch.msgid.link/20261025204807.122875-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/btrfs/verity.c | 7 ++++--- fs/ext4/verity.c | 4 +++- fs/verity/enable.c | 12 +++++------- include/linux/fsverity.h | 12 +++++++----- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c index ccfa2ef97e50..b34d2247cf84 100644 --- a/fs/btrfs/verity.c +++ b/fs/btrfs/verity.c @@ -483,8 +483,6 @@ static int rollback_verity(struct btrfs_inode *inode) inode->vfs_inode.i_ino); goto out; } - inode->ro_flags &= ~BTRFS_INODE_RO_VERITY; - btrfs_sync_inode_flags_to_i_flags(inode); ret = btrfs_update_inode(trans, inode); if (unlikely(ret)) { btrfs_abort_transaction(trans, ret); @@ -549,16 +547,19 @@ static int finish_verity(struct btrfs_inode *inode, const void *desc, if (IS_ERR(trans)) return PTR_ERR(trans); inode->ro_flags |= BTRFS_INODE_RO_VERITY; - btrfs_sync_inode_flags_to_i_flags(inode); ret = btrfs_update_inode(trans, inode); if (ret) goto end_trans; ret = del_orphan(trans, inode); if (ret) goto end_trans; + /* Now that rollback is no longer possible, set S_VERITY. */ + btrfs_sync_inode_flags_to_i_flags(inode); clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags); btrfs_set_fs_compat_ro(root->fs_info, VERITY); end_trans: + if (ret) + inode->ro_flags &= ~BTRFS_INODE_RO_VERITY; btrfs_end_transaction(trans); return ret; diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c index ac5c133f5529..56fe3e5b33e0 100644 --- a/fs/ext4/verity.c +++ b/fs/ext4/verity.c @@ -216,17 +216,19 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc, goto stop_and_cleanup; ext4_set_inode_flag(inode, EXT4_INODE_VERITY); - ext4_set_inode_flags(inode, false); err = ext4_mark_iloc_dirty(handle, inode, &iloc); if (err) goto stop_and_cleanup; ext4_journal_stop(handle); + /* Now that rollback is no longer possible, set S_VERITY. */ + ext4_set_inode_flags(inode, false); ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS); return 0; stop_and_cleanup: + ext4_clear_inode_flag(inode, EXT4_INODE_VERITY); ext4_journal_stop(handle); cleanup: /* diff --git a/fs/verity/enable.c b/fs/verity/enable.c index 2949fb8afb2f..fe8493280b77 100644 --- a/fs/verity/enable.c +++ b/fs/verity/enable.c @@ -276,8 +276,8 @@ static int enable_verity(struct file *filp, * initialization so that we don't have to undo the enabling when memory * allocation for the hash table fails. This is safe because looking up * the fsverity_info always first checks the S_VERITY flag on the inode, - * which will only be set at the very end of the ->end_enable_verity - * method. + * which will only be set at the very end of a successful call to + * ->end_enable_verity(). */ err = fsverity_set_info(vi); if (err) { @@ -287,10 +287,7 @@ static int enable_verity(struct file *filp, /* * Tell the filesystem to finish enabling verity on the file. - * Serialized with ->begin_enable_verity() by the inode lock. The file - * system needs to set the S_VERITY flag on the inode at the very end of - * the method, at which point the fsverity information can be accessed - * by other threads. + * Serialized with ->begin_enable_verity() by the inode lock. */ inode_lock(inode); err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size); @@ -298,7 +295,8 @@ static int enable_verity(struct file *filp, if (err) { fsverity_err(inode, "%ps() failed with err %d", vops->end_enable_verity, err); - fsverity_remove_info(vi); + if (!WARN_ON_ONCE(IS_VERITY(inode))) + fsverity_remove_info(vi); } else if (WARN_ON_ONCE(!IS_VERITY(inode))) { fsverity_remove_info(vi); err = -EINVAL; diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h index f9433332c274..61fe96ccdff6 100644 --- a/include/linux/fsverity.h +++ b/include/linux/fsverity.h @@ -54,11 +54,13 @@ struct fsverity_operations { * @merkle_tree_size: total bytes the Merkle tree took up * * If desc == NULL, then enabling verity failed and the filesystem only - * must do any necessary cleanups. Else, it must also store the given - * verity descriptor to a fs-specific location associated with the inode - * and do any fs-specific actions needed to mark the inode as a verity - * inode, e.g. setting a bit in the on-disk inode. The filesystem is - * also responsible for setting the S_VERITY flag in the VFS inode. + * must do any necessary cleanups. + * + * Else, the filesystem must store the given verity descriptor to a + * fs-specific location associated with the inode and do any fs-specific + * actions needed to mark the inode as a verity inode, e.g. set a bit in + * the on-disk inode. Finally, on success (only when rollback is no + * longer possible!) it must set S_VERITY in the VFS inode. * * i_rwsem is held for write, but it may have been dropped between * ->begin_enable_verity() and ->end_enable_verity().