linux-kernel - Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9c25c201f384d7320fd83e648d61a0086016ee36.camel@huaweicloud.com>
Date: Wed, 04 Feb 2026 13:34:09 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Frederick Lawler <fred@...udflare.com>, Mimi Zohar
 <zohar@...ux.ibm.com>,  Roberto Sassu <roberto.sassu@...wei.com>, Dmitry
 Kasatkin <dmitry.kasatkin@...il.com>, Eric Snowberg
 <eric.snowberg@...cle.com>, Paul Moore <paul@...l-moore.com>, James Morris
 <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>, "Darrick J.
 Wong" <djwong@...nel.org>, Christian Brauner <brauner@...nel.org>, Josef
 Bacik <josef@...icpanda.com>, Jeff Layton <jlayton@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-integrity@...r.kernel.org, 
	linux-security-module@...r.kernel.org, kernel-team@...udflare.com
Subject: Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call
 into vfs

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@...udflare.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;
>  		}
>