On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> introduced a means to track change detection for an inode
> via ctime updates, opposed to setting kstat.change_cookie to
> an i_version when calling into xfs_vn_getattr().
>
> This introduced a regression for IMA such that an action
> performed on a LOWER inode on a stacked file systems always
> requires a re-evaluation if the LOWER file system does not
> leverage kstat.change_cookie to track inode i_version or lacks
> i_version support all together.
>
> In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> leverages generic_fillattr() to update the change cookie.
>
> This is because IMA caches kstat.change_cookie to compare against an
> inode's i_version directly in integrity_inode_attrs_changed(), and thus
> could be out of date depending on how file systems set
> kstat.change_cookie.
>
> To address this, require integrity_inode_attrs_changed() to query
> vfs_getattr_nosec() to compare the cached version against
> kstat.change_cookie directly. This ensures that when updates occur,
> we're accessing the same changed inode version on changes, and fallback
> to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> result mask.
>
> Lastly, because EVM still relies on querying and caching a inode's
> i_version directly, the integrity_inode_attrs_changed() falls back to the
> original inode.i_version != cached comparison.
>
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Suggested-by: Jeff Layton <[email protected]>
> Signed-off-by: Frederick Lawler <[email protected]>
> ---
> We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> using multigrain ctime (as well as other file systems) for
> change detection in commit 1cf7e834a6fb ("xfs: switch to
> multigrain timestamps").
>
> Because file systems may implement i_version as they see fit, IMA
> caching may be behind as well as file systems that don't support/export
> i_version. Thus we're proposing to compare against the kstat.change_cookie
> directly to the cached version, and fall back to a ctime guard when
> that's not updated.
>
> EVM is largely left alone since there's no trivial way to query a file
> directly in the LSM call paths to obtain kstat.change_cookie &
> kstat.ctime to cache. Thus retains accessing i_version directly.
>
> Regression tests will be added to the Linux Test Project instead of
> selftest to help catch future file system changes that may impact
> future evaluation of IMA.
>
> I'd like this to be backported to at least 6.18 if possible.
>
> Below is a simplified test that demonstrates the issue:
>
> _fragment.config_
> CONFIG_XFS_FS=y
> CONFIG_OVERLAY_FS=y
> CONFIG_IMA=y
> CONFIG_IMA_WRITE_POLICY=y
> CONFIG_IMA_READ_POLICY=y
>
> _./test.sh_
>
> IMA_POLICY="/sys/kernel/security/ima/policy"
> TEST_BIN="/bin/date"
> MNT_BASE="/tmp/ima_test_root"
>
> mkdir -p "$MNT_BASE"
> mount -t tmpfs tmpfs "$MNT_BASE"
> mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
>
> dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> mkfs.xfs -q "$MNT_BASE/xfs.img"
> mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
>
> mount -t overlay overlay -o \
> "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work"
> \
> "$MNT_BASE/ovl"
>
> echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
>
> target_prog="$MNT_BASE/ovl/test_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
>
> audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
>
> if [[ "$audit_count" -eq 1 ]]; then
> echo "PASS: Found exactly 1 audit event."
> else
> echo "FAIL: Expected 1 audit event, but found $audit_count."
> exit 1
> fi
> ---
> Changes since RFC:
> - Remove calls to I_IS_VERSION()
> - Function documentation/comments
> - Abide IMA/EVM change detection fallback invariants
> - Combined ctime guard into version for attributes struct
> - Link to RFC:
> https://lore.kernel.org/r/[email protected]
> ---
> Changes in v2:
> - Updated commit description + message to clarify the problem.
> - compare struct timespec64 to avoid collision possibility [Roberto].
> - Don't check inode_attr_changed() in ima_check_last_writer()
> - Link to v1:
> https://lore.kernel.org/r/[email protected]
> ---
> include/linux/integrity.h | 40
> ++++++++++++++++++++++++++++++++-----
> security/integrity/evm/evm_crypto.c | 4 +++-
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_api.c | 20 +++++++++++++------
> security/integrity/ima/ima_main.c | 18 ++++++++++-------
> 5 files changed, 65 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index
> f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7
> 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -9,6 +9,8 @@
>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> +#include <linux/kernel.h>
> +#include <linux/time64.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
>
> /* An inode's attributes for detection of changes */
> struct integrity_inode_attributes {
> + struct timespec64 ctime;
I found the helper timespec64_to_ns(), I think it would be better for
memory occupation perspective to fit in the version field.
Thanks
Roberto
> u64 version; /* track inode changes */
> unsigned long ino;
> dev_t dev;
> @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> */
> static inline void
> integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> - u64 i_version, const struct inode *inode)
> + u64 i_version, struct timespec64 ctime,
> + const struct inode *inode)
> {
> + attrs->ctime = ctime;
> attrs->version = i_version;
> attrs->dev = inode->i_sb->s_dev;
> attrs->ino = inode->i_ino;
> @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct
> integrity_inode_attributes *attrs,
>
> /*
> * On stacked filesystems detect whether the inode or its content has
> changed.
> + *
> + * Must be called in process context.
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + might_sleep();
> +
> + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> + return true;
> +
> + /*
> + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> + * is not a good indicator of i_version support, this still retains
> + * the logic such that a re-evaluation should still occur for EVM, and
> + * only for IMA if vfs_getattr_nosec() fails.
> + */
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT))
> + return !IS_I_VERSION(inode) ||
> + !inode_eq_iversion(inode, attrs->version);
> +
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + return stat.change_cookie != attrs->version;
> +
> + if (stat.result_mask & STATX_CTIME)
> + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> +
> + return true;
> }
>
>
> diff --git a/security/integrity/evm/evm_crypto.c
> b/security/integrity/evm/evm_crypto.c
> index
> a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe
> 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> hmac_add_misc(desc, inode, type, data->digest);
>
> if (inode != d_backing_inode(dentry) && iint) {
> + struct timespec64 ctime = {0};
> +
> if (IS_I_VERSION(inode))
> i_version = inode_query_iversion(inode);
> integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> - inode);
> + ctime, inode);
> }
>
> /* Portable EVM signatures must include an IMA hash */
> diff --git a/security/integrity/evm/evm_main.c
> b/security/integrity/evm/evm_main.c
> index
> 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6
> 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct
> inode *metadata_inode)
> bool ret = false;
>
> if (iint) {
> - ret = (!IS_I_VERSION(metadata_inode) ||
> - integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> + NULL, metadata_inode);
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c
> b/security/integrity/ima/ima_api.c
> index
> c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585
> 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint,
> struct file *file,
> int length;
> void *tmpbuf;
> u64 i_version = 0;
> + struct timespec64 ctime = {0};
>
> /*
> * Always collect the modsig, because IMA might have already collected
> @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache
> *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + if (stat.result_mask & STATX_CTIME)
> + ctime = stat.ctime;
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache
> *iint, struct file *file,
>
> iint->ima_hash = tmpbuf;
> memcpy(iint->ima_hash, &hash, length);
> - if (real_inode == inode)
> + if (real_inode == inode) {
> iint->real_inode.version = i_version;
> - else
> + iint->real_inode.ctime = ctime;
> + } else {
> integrity_inode_attrs_store(&iint->real_inode, i_version,
> - real_inode);
> + ctime, real_inode);
> + }
>
> /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> if (!result)
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index
> 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b
> 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -22,12 +22,14 @@
> #include <linux/mount.h>
> #include <linux/mman.h>
> #include <linux/slab.h>
> +#include <linux/stat.h>
> #include <linux/xattr.h>
> #include <linux/ima.h>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> #include <linux/evm.h>
> #include <linux/crash_dump.h>
> +#include <linux/time64.h>
>
> #include "ima.h"
>
> @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache
> *iint,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> - AT_STATX_SYNC_AS_STAT) ||
> - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> - stat.change_cookie != iint->real_inode.version) {
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT) ||
> + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> + stat.change_cookie != iint->real_inode.version :
> + (!(stat.result_mask & STATX_CTIME) ||
> + !timespec64_equal(&stat.ctime,
> + &iint->real_inode.ctime)))) {
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
> real_inode = d_real_inode(file_dentry(file));
> if (real_inode != inode &&
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> - if (!IS_I_VERSION(real_inode) ||
> - integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + if (integrity_inode_attrs_changed(&iint->real_inode,
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251212-xfs-ima-fixup-931780a62c2c
>
> Best regards,
