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

[Mimi Zohar]

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?

Mimi

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