* [PATCH 5.10 1/2] fs-verity: factor out fsverity_get_descriptor()
2026-11-07 19:44 [PATCH 5.10 0/2] Backport additional ->get_verity_descriptor() checks Eric Biggers
@ 2026-11-07 19:44 ` Eric Biggers
2026-11-07 19:44 ` [PATCH 5.10 2/2] fsverity: Be more careful with return value of ->get_verity_descriptor() Eric Biggers
1 sibling, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2026-11-07 19:44 UTC (permalink / raw)
To: stable; +Cc: fsverity, Eric Biggers, Victor Hsieh, Jaegeuk Kim, Chao Yu
From: Eric Biggers <ebiggers@google.com>
commit c2c8261151b32f1956fc4ecd71c9a3e7972084b6 upstream.
The FS_IOC_READ_VERITY_METADATA ioctl will need to return the fs-verity
descriptor (and signature) to userspace.
There are a few ways we could implement this:
- Save a copy of the descriptor (and signature) in the fsverity_info
struct that hangs off of the in-memory inode. However, this would
waste memory since most of the time it wouldn't be needed.
- Regenerate the descriptor from the merkle_tree_params in the
fsverity_info. However, this wouldn't work for the signature, nor for
the salt which the merkle_tree_params only contains indirectly as part
of the 'hashstate'. It would also be error-prone.
- Just get them from the filesystem again. The disadvantage is that in
general we can't trust that they haven't been maliciously changed
since the file has opened. However, the use cases for
FS_IOC_READ_VERITY_METADATA don't require that it verifies the chain
of trust. So this is okay as long as we do some basic validation.
In preparation for implementing the third option, factor out a helper
function fsverity_get_descriptor() which gets the descriptor (and
appended signature) from the filesystem and does some basic validation.
As part of this, start checking the sig_size field for overflow.
Currently fsverity_verify_signature() does this. But the new ioctl will
need this too, so do it earlier.
Link: https://lore.kernel.org/r/20210115181819.34732-2-ebiggers@kernel.org
Reviewed-by: Victor Hsieh <victorhsieh@google.com>
Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/verity/fsverity_private.h | 7 +-
fs/verity/open.c | 130 +++++++++++++++++++++++------------
2 files changed, 91 insertions(+), 46 deletions(-)
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index e96d99d5145e1..08cd74c193d40 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -152,12 +152,17 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,
const u8 *salt, size_t salt_size);
struct fsverity_info *fsverity_create_info(const struct inode *inode,
- void *desc, size_t desc_size);
+ struct fsverity_descriptor *desc,
+ size_t desc_size);
void fsverity_set_info(struct inode *inode, struct fsverity_info *vi);
void fsverity_free_info(struct fsverity_info *vi);
+int fsverity_get_descriptor(struct inode *inode,
+ struct fsverity_descriptor **desc_ret,
+ size_t *desc_size_ret);
+
int __init fsverity_init_info_cache(void);
void __init fsverity_exit_info_cache(void);
diff --git a/fs/verity/open.c b/fs/verity/open.c
index 67d71f7b1b483..804bd717c4d32 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -142,45 +142,17 @@ static int compute_file_measurement(struct fsverity_hash_alg *hash_alg,
}
/*
- * Validate the given fsverity_descriptor and create a new fsverity_info from
- * it. The signature (if present) is also checked.
+ * Create a new fsverity_info from the given fsverity_descriptor (with optional
+ * appended signature), and check the signature if present. The
+ * fsverity_descriptor must have already undergone basic validation.
*/
struct fsverity_info *fsverity_create_info(const struct inode *inode,
- void *_desc, size_t desc_size)
+ struct fsverity_descriptor *desc,
+ size_t desc_size)
{
- struct fsverity_descriptor *desc = _desc;
struct fsverity_info *vi;
int err;
- if (desc_size < sizeof(*desc)) {
- fsverity_err(inode, "Unrecognized descriptor size: %zu bytes",
- desc_size);
- return ERR_PTR(-EINVAL);
- }
-
- if (desc->version != 1) {
- fsverity_err(inode, "Unrecognized descriptor version: %u",
- desc->version);
- return ERR_PTR(-EINVAL);
- }
-
- if (memchr_inv(desc->__reserved, 0, sizeof(desc->__reserved))) {
- fsverity_err(inode, "Reserved bits set in descriptor");
- return ERR_PTR(-EINVAL);
- }
-
- if (desc->salt_size > sizeof(desc->salt)) {
- fsverity_err(inode, "Invalid salt_size: %u", desc->salt_size);
- return ERR_PTR(-EINVAL);
- }
-
- if (le64_to_cpu(desc->data_size) != inode->i_size) {
- fsverity_err(inode,
- "Wrong data_size: %llu (desc) != %lld (inode)",
- le64_to_cpu(desc->data_size), inode->i_size);
- return ERR_PTR(-EINVAL);
- }
-
vi = kmem_cache_zalloc(fsverity_info_cachep, GFP_KERNEL);
if (!vi)
return ERR_PTR(-ENOMEM);
@@ -245,15 +217,57 @@ void fsverity_free_info(struct fsverity_info *vi)
kmem_cache_free(fsverity_info_cachep, vi);
}
-/* Ensure the inode has an ->i_verity_info */
-static int ensure_verity_info(struct inode *inode)
+static bool validate_fsverity_descriptor(struct inode *inode,
+ const struct fsverity_descriptor *desc,
+ size_t desc_size)
{
- struct fsverity_info *vi = fsverity_get_info(inode);
- struct fsverity_descriptor *desc;
- int res;
+ if (desc_size < sizeof(*desc)) {
+ fsverity_err(inode, "Unrecognized descriptor size: %zu bytes",
+ desc_size);
+ return false;
+ }
- if (vi)
- return 0;
+ if (desc->version != 1) {
+ fsverity_err(inode, "Unrecognized descriptor version: %u",
+ desc->version);
+ return false;
+ }
+
+ if (memchr_inv(desc->__reserved, 0, sizeof(desc->__reserved))) {
+ fsverity_err(inode, "Reserved bits set in descriptor");
+ return false;
+ }
+
+ if (desc->salt_size > sizeof(desc->salt)) {
+ fsverity_err(inode, "Invalid salt_size: %u", desc->salt_size);
+ return false;
+ }
+
+ if (le64_to_cpu(desc->data_size) != inode->i_size) {
+ fsverity_err(inode,
+ "Wrong data_size: %llu (desc) != %lld (inode)",
+ le64_to_cpu(desc->data_size), inode->i_size);
+ return false;
+ }
+
+ if (le32_to_cpu(desc->sig_size) > desc_size - sizeof(*desc)) {
+ fsverity_err(inode, "Signature overflows verity descriptor");
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * Read the inode's fsverity_descriptor (with optional appended signature) from
+ * the filesystem, and do basic validation of it.
+ */
+int fsverity_get_descriptor(struct inode *inode,
+ struct fsverity_descriptor **desc_ret,
+ size_t *desc_size_ret)
+{
+ int res;
+ struct fsverity_descriptor *desc;
res = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0);
if (res < 0) {
@@ -272,20 +286,46 @@ static int ensure_verity_info(struct inode *inode)
res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, res);
if (res < 0) {
fsverity_err(inode, "Error %d reading verity descriptor", res);
- goto out_free_desc;
+ kfree(desc);
+ return res;
+ }
+
+ if (!validate_fsverity_descriptor(inode, desc, res)) {
+ kfree(desc);
+ return -EINVAL;
}
- vi = fsverity_create_info(inode, desc, res);
+ *desc_ret = desc;
+ *desc_size_ret = res;
+ return 0;
+}
+
+/* Ensure the inode has an ->i_verity_info */
+static int ensure_verity_info(struct inode *inode)
+{
+ struct fsverity_info *vi = fsverity_get_info(inode);
+ struct fsverity_descriptor *desc;
+ size_t desc_size;
+ int err;
+
+ if (vi)
+ return 0;
+
+ err = fsverity_get_descriptor(inode, &desc, &desc_size);
+ if (err)
+ return err;
+
+ vi = fsverity_create_info(inode, desc, desc_size);
if (IS_ERR(vi)) {
- res = PTR_ERR(vi);
+ err = PTR_ERR(vi);
goto out_free_desc;
}
fsverity_set_info(inode, vi);
- res = 0;
+ err = 0;
out_free_desc:
kfree(desc);
- return res;
+ return err;
}
/**
--
2.56.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 5.10 2/2] fsverity: Be more careful with return value of ->get_verity_descriptor()
2026-11-07 19:44 [PATCH 5.10 0/2] Backport additional ->get_verity_descriptor() checks Eric Biggers
2026-11-07 19:44 ` [PATCH 5.10 1/2] fs-verity: factor out fsverity_get_descriptor() Eric Biggers
@ 2026-11-07 19:44 ` Eric Biggers
1 sibling, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2026-11-07 19:44 UTC (permalink / raw)
To: stable; +Cc: fsverity, Eric Biggers
commit 8f8faec82aa902b1997ae578c46bcc84afbad811 upstream.
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 <ebiggers@kernel.org>
---
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 804bd717c4d32..7a786126a1340 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -221,12 +221,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);
@@ -266,31 +260,37 @@ int fsverity_get_descriptor(struct inode *inode,
struct fsverity_descriptor **desc_ret,
size_t *desc_size_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;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 3+ messages in thread