On Wed, Feb 04, 2026 at 01:34:09PM +0100, Roberto Sassu wrote: > On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote: > > Align integrity_inode_attrs_changed() to ima_check_last_writer()'s > > semantics when detecting changes. > > > > For IMA, stacked file systems that do not set kstat.change_cookie, > > integrity_inode_attrs_changed() will compare zero to zero, thus no > > I setup overlay with two xfs filesystems, kept the file I want to be > audited in the lower filesystem. > > Without this patch set, if I modify the lower file, changes are > detected, because actually the i_version is incremented.
Correct, but the test example in 00/03 demonstrates that there's no modification. For workloads that only execute and not modify, IMA shouldn't evaluate more than once, but that's what we're observing at least for XFS. > > In which situation there is a comparison zero to zero? My mistake. You're right, but for the wrong reason. To be clear, these patches are about the STATX_CHANGE_COOKIE mechanic. XFS updates the i_version regardless of the multigrain ctime changes. You're correct in that with/without this patch there is no zero-zero comparison for XFS, and that's because XFS isn't setting the STATX_CHANGE_COOKIE in the result mask either for last writer check or the attrs changed check, thus a change is always detected with integrity_inode_attrs_stat_changed(), and thus maintains current IMA behavior for XFS. That said, should a file system set STATX_CHANGE_COOKIE in the result mask, and not update the i_version (say its kept at zero), then it's always zero-zero. I don't know how likely that scenario is. I should reword this commit, but I am a bit hesitant to say "don't squash this in with patch 3" due to that uncertainty. > > Thanks > > Roberto > > > change detected. This is not dissimilar to what > > ima_check_last_writer() does. > > > > No logical change intended for EVM. > > > > Signed-off-by: Frederick Lawler <[email protected]> > > --- > > include/linux/integrity.h | 28 ++++++++++++++++++++++++---- > > security/integrity/evm/evm_main.c | 5 ++--- > > security/integrity/ima/ima_main.c | 5 ++--- > > 3 files changed, 28 insertions(+), 10 deletions(-) > > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h > > index > > beb9ab19fa6257e79266b58bcb5f55b0c5445828..382c783f0fa3ae4a938cdf9559291ba1903a378e > > 100644 > > --- a/include/linux/integrity.h > > +++ b/include/linux/integrity.h > > @@ -9,6 +9,7 @@ > > > > #include <linux/fs.h> > > #include <linux/iversion.h> > > +#include <linux/kernel.h> > > > > enum integrity_status { > > INTEGRITY_PASS = 0, > > @@ -62,14 +63,33 @@ integrity_inode_attrs_stat_changed > > > > /* > > * 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, > > + AT_STATX_SYNC_AS_STAT)) > > + return !IS_I_VERSION(inode) || > > + !inode_eq_iversion(inode, attrs->version); > > + > > + return integrity_inode_attrs_stat_changed(attrs, &stat); > > } > > > > > > 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_main.c > > b/security/integrity/ima/ima_main.c > > index > > 6570ad10887b9ea1172c78274cf62482350e87ff..8cb17c9d446caaa5a98f5ec8f027c17ba7babca8 > > 100644 > > --- a/security/integrity/ima/ima_main.c > > +++ b/security/integrity/ima/ima_main.c > > @@ -328,9 +328,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; > > } > > >
