[PATCH RFC] ima: Fallback to a ctime guard without i_version updates

[Frederick Lawler]

On Mon, Jan 12, 2026 at 09:02:02AM -0500, Mimi Zohar wrote:
> On Tue, 2026-01-06 at 14:50 -0500, Jeff Layton wrote:
> > > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > >    */
> > > > > >   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;
> > > > > > +
> > > > > > +	if (inode->i_sb->s_dev != attrs->dev ||
> > > > > > +	    inode->i_ino != attrs->ino)
> > > > > > +		return true;
> > > > > > +
> > > > > > +	if (inode_eq_iversion(inode, attrs->version))
> > > > > > +		return false;
> > > > > > +
> > > > > > +	if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > > +				       AT_STATX_SYNC_AS_STAT))
> > > > > > +		return true;
> > > > > > +
> > > > > 
> > > > > This is rather odd. You're sampling the i_version field directly, but
> > > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > > > 
> > > > > It's particularly odd since you don't know whether the i_version field
> > > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > > i_version field generally stays at 0, so won't this never fall through
> > > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > > > 
> > > > 
> > > > You're totally right. I didn't consider FS's caching the value at zero.
> > > 
> > > Actually, I'm going to amend this. I think I did consider FSs without an
> > > implementation. Where this is called at, it is often guarded by a
> > > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > > understanding this correctly, the check call doesn't occur unless the inode
> > > has i_version support.
> > > 
> > 
> > 
> > It depends on what you mean by i_version support:
> > 
> > That flag just tells the VFS that it needs to bump the i_version field
> > when updating timestamps. It's not a reliable indicator of whether the
> > i_version field is suitable for the purpose you want here.
> > 
> > The problem here and the one that we ultimately fixed with multigrain
> > timestamps is that XFS in particular will bump i_version on any change
> > to the log. That includes atime updates due to reads.
> > 
> > XFS still tracks the i_version the way it always has, but we've stopped
> > getattr() from reporting it because it's not suitable for the purpose
> > that nfsd (and IMA) need it for.
> > 
> > > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > > checks guarding the call sites, grab both ctime and cookie from stat,
> > > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > > against the cached i_version with one of those values, and then fall
> > > back to ctime?
> > > 
> > 
> > Not exactly.
> > 
> > You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> > then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> > use that. If it's not then use the ctime.
> > 
> > The part I'm not sure about is whether it's actually safe to do this.
> > vfs_getattr_nosec() can block in some situations. Is it ok to do this
> > in any context where integrity_inode_attrs_changed() may be called? 
> 
> Frederick, before making any changes, please describe the problem you're
> actually seeing. From my limited testing, file change IS being detected. A major
> change like Jeff is suggesting is not something that would or should be back
> ported.  Remember, Jeff's interest is remote filesystems, not necessarily with
> your particular XFS concern.
> 
> So again, what is the problem you're trying to address?

It's easier if I paste a simpler version of test I've been promising
for v1 to help show this (below).

In 6.12 the test snippet passes, for >= 6.13 we get an audit
evaluation on the each execution when there should only be 1.

The struct integrity_inode_attributes.version stays at zero for XFS
in the below test, as well as file systems that calls into
generic_fillattr() or otherwise that doesn't set the change cookie
request mask.

When file systems have a mutated file, the cookie is then updated,
but the compare against inode.i_version could be out of date depending
on file system implementation. Thus we see since 6.13, XFS an atime change
will cause another evaluation due to stale cache.

I'm not expecting a backport to 6.13 as there has been a lot of changes
in IMA/EVM, but I think to the 6.18 LTS is reasonable. With leaving
EVM alone, it's a small diff.

I have a updated patch that hopefully addresses all your concerns
from other responses in this thread. I want to point out that the updated
code is more EVM/IMA invariant mindful than this RFC. I'd
like to submit that, and then move discussion over there if possible.

Hopefully this helps,
Fred

_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_
#!/bin/bash -e

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

> 
> Mimi
> 
> > 
> > ISTR that this was an issue at one point, but maybe isn't now that IMA
> > is an LSM?
>