* Re: overlayfs: verity validation broken since f77f281b6118
[not found] ` <afXCoGk2Bu9rBKvJ@amir-ThinkPad-T480>
@ 2026-05-05 16:51 ` Colin Walters
2026-05-05 17:18 ` Eric Biggers
0 siblings, 1 reply; 6+ messages in thread
From: Colin Walters @ 2026-05-05 16:51 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christoph Hellwig, Eric Biggers, linux-fsdevel@vger.kernel.org,
fsverity
On Sat, May 2, 2026, at 5:23 AM, Amir Goldstein wrote:
> On Fri, May 01, 2026 at 01:14:54PM -0400, Colin Walters wrote:
>> Hi Christoph & Eric,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f77f281b6118 broke composefs's usage of overlayfs verity=require, this was reported originally in https://github.com/bootc-dev/bootc/issues/2174
>>
>> There's some output from an agent run I had in the <details> there, but here's an xfstests patch that passes on without that commit and fails with it.
>>
>> From 14231122bfd1e41337e4fb847acbbe038457c32a Mon Sep 17 00:00:00 2001
>> From: Colin Walters <walters@verbum.org>
>> Date: Fri, 1 May 2026 09:45:58 -0400
>> Subject: [PATCH] overlay/118: test fsverity lazy load through metacopy overlay
>>
>> Reproduces the regression reported at:
>> https://github.com/bootc-dev/bootc/issues/2174
>>
>> A recent change in how fsverity state was cached in memory
>> I think caused inodes not in cache to appear to have
>> missing verity=require for overlayfs.
>>
>> This test catches that.
>>
>> Generated-by: OpenCode (Claude Sonnet 4.5)
>> Signed-off-by: Colin Walters <walters@verbum.org>
>> ---
>> tests/overlay/118 | 62 +++++++++++++++++++++++++++++++++++++++++++
>> tests/overlay/118.out | 1 +
>
>
> Please use free test numbers below 100
OK, I can resend with that change if that's the only thing.
>
> Is there a kernel fix for this? please mention it.
Not that I know of. I did have my agent framework (opencode + combo of Gemini+Claude models) generate one initially, but I intentionally didn't post it because the generating is ~easy, verifying it's "good" is another thing and my C has bitrotted a bit (in favor of Rust mostly but I have to deal with a lot of Go too).
Anyways, this trivial change works:
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 13cb60b52bd6..af8f6c2989ce 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -1039,7 +1039,7 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
if (WARN_ON_ONCE(lowerdata.dentry == NULL) ||
ovl_ensure_verity_loaded(&lowerdata) ||
- !fsverity_active(d_inode(lowerdata.dentry))) {
+ !fsverity_get_info(d_inode(lowerdata.dentry))) {
return false;
}
}
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 7b86a6bac644..bfdf9310ee78 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1354,7 +1354,7 @@ int ovl_ensure_verity_loaded(const struct path *datapath)
struct inode *inode = d_inode(datapath->dentry);
struct file *filp;
- if (!fsverity_active(inode) && IS_VERITY(inode)) {
+ if (IS_VERITY(inode) && !fsverity_get_info(inode)) {
/*
* If this inode was not yet opened, the verity info hasn't been
* loaded yet, so we need to do that here to force it into memory.
However, when I looked at this more closely I felt the APIs are too confusing, and my Rust-ified mindset especially didn't like the side-effectful nature of ovl_ensure_verity_loaded().
So here's a much bigger patch that looks a lot nicer to me upon a quick local review:
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index af8f6c2989ce..9d3908666b7e 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -1037,9 +1037,11 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
ovl_path_lowerdata(dentry, &lowerdata);
+ struct inode *inode = d_inode(lowerdata.dentry);
+
if (WARN_ON_ONCE(lowerdata.dentry == NULL) ||
- ovl_ensure_verity_loaded(&lowerdata) ||
- !fsverity_get_info(d_inode(lowerdata.dentry))) {
+ !IS_VERITY(inode) ||
+ IS_ERR(fsverity_get_or_load_info(inode))) {
return false;
}
}
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index b75df37f70ac..4d23ff29a4c3 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -581,7 +581,6 @@ int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
struct ovl_metacopy *metacopy);
bool ovl_is_metacopy_dentry(struct dentry *dentry);
char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding);
-int ovl_ensure_verity_loaded(const struct path *path);
int ovl_validate_verity(struct ovl_fs *ofs,
const struct path *metapath,
const struct path *datapath);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index bfdf9310ee78..99aa2312de39 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1348,25 +1348,6 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa
return ERR_PTR(res);
}
-/* Call with mounter creds as it may open the file */
-int ovl_ensure_verity_loaded(const struct path *datapath)
-{
- struct inode *inode = d_inode(datapath->dentry);
- struct file *filp;
-
- if (IS_VERITY(inode) && !fsverity_get_info(inode)) {
- /*
- * If this inode was not yet opened, the verity info hasn't been
- * loaded yet, so we need to do that here to force it into memory.
- */
- filp = kernel_file_open(datapath, O_RDONLY, current_cred());
- if (IS_ERR(filp))
- return PTR_ERR(filp);
- fput(filp);
- }
-
- return 0;
-}
int ovl_validate_verity(struct ovl_fs *ofs,
const struct path *metapath,
@@ -1375,7 +1356,7 @@ int ovl_validate_verity(struct ovl_fs *ofs,
struct ovl_metacopy metacopy_data;
u8 actual_digest[FS_VERITY_MAX_DIGEST_SIZE];
int xattr_digest_size, digest_size;
- int xattr_size, err;
+ int xattr_size;
u8 verity_algo;
if (!ofs->config.verity_mode ||
@@ -1398,16 +1379,9 @@ int ovl_validate_verity(struct ovl_fs *ofs,
xattr_digest_size = ovl_metadata_digest_size(&metacopy_data);
- err = ovl_ensure_verity_loaded(datapath);
- if (err < 0) {
- pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
- datapath->dentry);
- return -EIO;
- }
-
digest_size = fsverity_get_digest(d_inode(datapath->dentry), actual_digest,
&verity_algo, NULL);
- if (digest_size == 0) {
+ if (digest_size <= 0) {
pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n", datapath->dentry);
return -EIO;
}
@@ -1426,21 +1400,14 @@ int ovl_validate_verity(struct ovl_fs *ofs,
int ovl_get_verity_digest(struct ovl_fs *ofs, const struct path *src,
struct ovl_metacopy *metacopy)
{
- int err, digest_size;
+ int digest_size;
if (!ofs->config.verity_mode || !S_ISREG(d_inode(src->dentry)->i_mode))
return 0;
- err = ovl_ensure_verity_loaded(src);
- if (err < 0) {
- pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
- src->dentry);
- return -EIO;
- }
-
digest_size = fsverity_get_digest(d_inode(src->dentry),
metacopy->digest, &metacopy->digest_algo, NULL);
- if (digest_size == 0 ||
+ if (digest_size <= 0 ||
WARN_ON_ONCE(digest_size > FS_VERITY_MAX_DIGEST_SIZE)) {
if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n",
diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index 6a35623ebdf0..4ccf2ab8a70a 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -68,9 +68,9 @@ EXPORT_SYMBOL_GPL(fsverity_ioctl_measure);
* @alg: (out) the digest's algorithm, as a FS_VERITY_HASH_ALG_* value
* @halg: (out) the digest's algorithm, as a HASH_ALGO_* value
*
- * Retrieves the fsverity digest of the given file. The file must have been
- * opened at least once since the inode was last loaded into the inode cache;
- * otherwise this function will not recognize when fsverity is enabled.
+ * Retrieves the fsverity digest of the given file. If the inode has the
+ * S_VERITY flag set but the fsverity_info has not yet been loaded into the
+ * in-memory hash table, this function will load it from disk automatically.
*
* The file's fsverity digest consists of @raw_digest in combination with either
* @alg or @halg. (The caller can choose which one of @alg or @halg to use.)
@@ -80,8 +80,8 @@ EXPORT_SYMBOL_GPL(fsverity_ioctl_measure);
* provides no security guarantee for users who ignore the algorithm ID, even if
* they use the digest size (since algorithms can share the same digest size).
*
- * Return: The size of the raw digest in bytes, or 0 if the file doesn't have
- * fsverity enabled.
+ * Return: The size of the raw digest in bytes, 0 if the file doesn't have
+ * fsverity enabled, or -errno on error.
*/
int fsverity_get_digest(struct inode *inode,
u8 raw_digest[FS_VERITY_MAX_DIGEST_SIZE],
@@ -90,10 +90,13 @@ int fsverity_get_digest(struct inode *inode,
const struct fsverity_info *vi;
const struct fsverity_hash_alg *hash_alg;
- vi = fsverity_get_info(inode);
- if (!vi)
+ if (!IS_VERITY(inode))
return 0; /* not a verity file */
+ vi = fsverity_get_or_load_info(inode);
+ if (IS_ERR(vi))
+ return PTR_ERR(vi);
+
hash_alg = vi->tree_params.hash_alg;
memcpy(raw_digest, vi->file_digest, hash_alg->digest_size);
if (alg)
@@ -116,6 +119,13 @@ __bpf_kfunc_start_defs();
*
* Read fsverity_digest of *file* into *digest_ptr*.
*
+ * If the file's fsverity_info has not yet been loaded into the in-memory hash
+ * table (i.e. the inode is "cold"), this function will read the verity
+ * descriptor from disk before returning. This may block on I/O. The
+ * function is restricted to BPF_PROG_TYPE_LSM hooks (sleepable context), so
+ * blocking is safe, but callers should be aware of the potential latency on
+ * the first call for a given inode.
+ *
* Return: 0 on success, a negative value on error.
*/
__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr *digest_p)
@@ -138,10 +148,13 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr *di
if (!IS_ALIGNED((uintptr_t)arg, __alignof__(*arg)))
return -EINVAL;
- vi = fsverity_get_info(inode);
- if (!vi)
+ if (!IS_VERITY(inode))
return -ENODATA; /* not a verity file */
+ vi = fsverity_get_or_load_info(inode);
+ if (IS_ERR(vi))
+ return PTR_ERR(vi);
+
hash_alg = vi->tree_params.hash_alg;
arg->digest_algorithm = hash_alg - fsverity_hash_algs;
diff --git a/fs/verity/open.c b/fs/verity/open.c
index dfa0d1afe0fe..4db71b396cdb 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -344,24 +344,48 @@ int fsverity_get_descriptor(struct inode *inode,
return 0;
}
-static int ensure_verity_info(struct inode *inode)
+/**
+ * fsverity_get_or_load_info() - get the fsverity_info for a verity inode,
+ * loading it from disk if necessary
+ * @inode: inode with the S_VERITY flag set
+ *
+ * If the fsverity_info has already been loaded into the in-memory hash table
+ * (i.e. the file has been opened), return it immediately. Otherwise read the
+ * verity descriptor from disk and populate the hash table entry.
+ *
+ * Unlike fsverity_get_info(), this function never returns NULL for a verity
+ * inode — it either returns the fsverity_info or an error.
+ *
+ * On encrypted verity files, the fscrypt key must already be set up on the
+ * inode (e.g. via a prior fscrypt_file_open()) before calling this function,
+ * as the descriptor read uses the existing crypto context rather than
+ * establishing one. Callers such as overlayfs that reach this path during
+ * copy-up already satisfy this precondition.
+ *
+ * The caller must ensure @inode has the S_VERITY flag set.
+ *
+ * Return: the fsverity_info on success, ERR_PTR(-errno) on failure
+ */
+struct fsverity_info *fsverity_get_or_load_info(struct inode *inode)
{
- struct fsverity_info *vi = fsverity_get_info(inode), *found;
+ struct fsverity_info *vi, *found;
struct fsverity_descriptor *desc;
int err;
+ if (WARN_ON_ONCE(!IS_VERITY(inode)))
+ return ERR_PTR(-EINVAL);
+
+ vi = fsverity_get_info(inode);
if (vi)
- return 0;
+ return vi;
err = fsverity_get_descriptor(inode, &desc);
if (err)
- return err;
+ return ERR_PTR(err);
vi = fsverity_create_info(inode, desc);
- if (IS_ERR(vi)) {
- err = PTR_ERR(vi);
+ if (IS_ERR(vi))
goto out_free_desc;
- }
/*
* Multiple tasks may race to set the inode's verity info, in which case
@@ -372,20 +396,20 @@ static int ensure_verity_info(struct inode *inode)
fsverity_info_hash_params);
if (found) {
fsverity_free_info(vi);
- if (IS_ERR(found))
- err = PTR_ERR(found);
+ vi = found; /* either the winner's entry or ERR_PTR */
}
out_free_desc:
kfree(desc);
- return err;
+ return vi;
}
+EXPORT_SYMBOL_GPL(fsverity_get_or_load_info);
int __fsverity_file_open(struct inode *inode, struct file *filp)
{
if (filp->f_mode & FMODE_WRITE)
return -EPERM;
- return ensure_verity_info(inode);
+ return PTR_ERR_OR_ZERO(fsverity_get_or_load_info(inode));
}
EXPORT_SYMBOL_GPL(__fsverity_file_open);
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index a8f9aa75b792..0ea8ef2348d2 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -136,13 +136,25 @@ struct fsverity_operations {
#ifdef CONFIG_FS_VERITY
/**
- * fsverity_active() - do reads from the inode need to go through fs-verity?
+ * fsverity_active() - does the inode have the S_VERITY flag set?
* @inode: inode to check
*
- * This checks whether the inode's verity info has been set, and reads need
- * to verify the file data.
+ * Check whether the inode has the S_VERITY flag set. This flag is set
+ * when the filesystem marks the inode as a verity file, and it persists
+ * for the lifetime of the inode in the inode cache.
*
- * Return: true if reads need to go through fs-verity, otherwise false
+ * Note: S_VERITY being set does NOT mean the fsverity_info has been loaded
+ * into the in-memory hash table yet. The info is loaded lazily on first
+ * open (via __fsverity_file_open()). Use fsverity_get_info() to check
+ * whether the info is loaded, or fsverity_get_or_load_info() to load it
+ * on demand.
+ *
+ * The smp_mb() pairs with the try_cmpxchg in set_mask_bits() that SETS the
+ * S_VERITY bit in i_flags. This ensures that stores made before the flag
+ * was set (in particular, the hash table insertion of the fsverity_info) are
+ * visible to any CPU that subsequently observes S_VERITY set.
+ *
+ * Return: true if the inode has S_VERITY set, otherwise false
*/
static inline bool fsverity_active(const struct inode *inode)
{
@@ -159,13 +171,26 @@ static inline bool fsverity_active(const struct inode *inode)
}
struct fsverity_info *__fsverity_get_info(const struct inode *inode);
+
/**
- * fsverity_get_info - get fsverity information for an inode
- * @inode: inode to operate on.
+ * fsverity_get_info() - get the in-memory fsverity_info for an inode
+ * @inode: inode to look up
*
- * This gets the fsverity_info for @inode if it exists. Safe to call without
- * knowin that a fsverity_info exist for @inode, including on file systems that
- * do not support fsverity.
+ * Return the fsverity_info for @inode if it has been loaded into the
+ * in-memory hash table, otherwise return NULL.
+ *
+ * A NULL return has two distinct meanings:
+ * 1. The inode is not a verity file at all (S_VERITY not set).
+ * 2. The inode IS a verity file, but its info has not been loaded yet
+ * because the file has not been opened since the inode was last evicted.
+ *
+ * Callers that need to distinguish these cases should check IS_VERITY(inode)
+ * alongside the return value. Callers that need the info to always be
+ * available should use fsverity_get_or_load_info() instead.
+ *
+ * Safe to call on any inode, even on filesystems that do not support fsverity.
+ *
+ * Return: the fsverity_info if loaded, NULL otherwise
*/
static inline struct fsverity_info *fsverity_get_info(const struct inode *inode)
{
@@ -187,6 +212,7 @@ int fsverity_get_digest(struct inode *inode,
/* open.c */
+struct fsverity_info *fsverity_get_or_load_info(struct inode *inode);
int __fsverity_file_open(struct inode *inode, struct file *filp);
/* read_metadata.c */
@@ -242,6 +268,11 @@ static inline int fsverity_get_digest(struct inode *inode,
/* open.c */
+static inline struct fsverity_info *fsverity_get_or_load_info(struct inode *inode)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
static inline int __fsverity_file_open(struct inode *inode, struct file *filp)
{
return -EOPNOTSUPP;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 0916f24f005f..1eabb2d76961 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -213,9 +213,14 @@ static bool ima_get_verity_digest(struct ima_iint_cache *iint,
/*
* On failure, 'measure' policy rules will result in a file data
* hash containing 0's.
+ *
+ * fsverity_get_digest() returns 0 if fsverity is not enabled on the
+ * file, or a negative errno if the verity info could not be loaded
+ * (e.g. -EIO reading the descriptor from disk). Treat both cases the
+ * same: fall back to the file data hash.
*/
digest_len = fsverity_get_digest(inode, hash->digest, NULL, &alg);
- if (digest_len == 0)
+ if (digest_len <= 0)
return false;
/*
diff --git a/security/ipe/eval.c b/security/ipe/eval.c
index 21439c5be336..1910f4b3641b 100644
--- a/security/ipe/eval.c
+++ b/security/ipe/eval.c
@@ -204,10 +204,10 @@ static bool evaluate_fsv_digest(const struct ipe_eval_ctx *const ctx,
if (!ctx->ino)
return false;
- if (!fsverity_get_digest((struct inode *)ctx->ino,
- digest,
- NULL,
- &alg))
+ if (fsverity_get_digest((struct inode *)ctx->ino,
+ digest,
+ NULL,
+ &alg) <= 0)
return false;
info.alg = hash_algo_name[alg];
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread