On Thu, 2026-01-22 at 10:49 -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 TMPFS 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.
>
It would be good to add:
Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
That will help the stable kernel automation pick this up too.
> 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 in v3:
> - Prefer timespec64_to_ns() to leverage attr.version. [Roberto]
> - s/TPMFS/TMPFS/ in description.
> - Link to v2:
> 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]
>
> 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]
> ---
> include/linux/integrity.h | 35 +++++++++++++++++++++++++++++++----
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_api.c | 11 ++++++++---
> security/integrity/ima/ima_main.c | 17 ++++++++++-------
> 4 files changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index
> f5842372359be5341b6870a43b92e695e8fc78af..034f0a1ed48ca8c19c764e302bbfc555dad92cde
> 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,
> @@ -51,14 +53,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_to_ns(&stat.ctime) != (s64)attrs->version;
> +
> + return true;
> }
>
>
> 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..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58
> 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -272,10 +272,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;
> + else if (stat.result_mask & STATX_CTIME)
> + i_version = timespec64_to_ns(&stat.ctime);
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index
> 5770cf691912aa912fc65280c59f5baac35dd725..8ac42b03740eb93bf23b15cb9039af6cd32aa999
> 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -28,6 +28,7 @@
> #include <linux/iversion.h>
> #include <linux/evm.h>
> #include <linux/crash_dump.h>
> +#include <linux/time64.h>
>
> #include "ima.h"
>
> @@ -199,10 +200,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_to_ns(&stat.ctime) !=
> + (s64)iint->real_inode.version))) {
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -328,9 +332,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,
I gave the IMA code a closer look this time. You can add:
Reviewed-by: Jeff Layton <[email protected]>
