[PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs

Frederick Lawler posted 3 patches 1 week, 2 days ago
[PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
Posted by Frederick Lawler 1 week, 2 days ago
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
change detected. This is not dissimilar to what
ima_check_last_writer() does.

No logical change intended for EVM.

Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
 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;
 		}

-- 
2.43.0
Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
Posted by Roberto Sassu 4 days, 21 hours ago
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.

In which situation there is a comparison zero to zero?

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 <fred@cloudflare.com>
> ---
>  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;
>  		}
> 
Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
Posted by Frederick Lawler 4 days, 14 hours ago
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 <fred@cloudflare.com>
> > ---
> >  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;
> >  		}
> > 
>