* [PATCH v5 0/3] ima: Detect changes to files via kstat changes rather than i_version
@ 2026-01-30 22:39 Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 1/3] ima: Unify vfs_getattr_nosec() stat comparisons under helper function Frederick Lawler
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Frederick Lawler @ 2026-01-30 22:39 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
We uncovered a case in kernels >= 6.13 where XFS is no longer updating
struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
using multigrain ctime (as well as other file systems) for
change detection in commit 1cf7e834a6fb ("xfs: switch to
multigrain timestamps").
Because file systems may implement i_version as they see fit, IMA
unnecessarily measures files in stacked file systems. This is due
to the LOWER or UPPER FS not updating kstat.change_cookie to the
recent i_version on request. Thus, for XFS, zero is being compared
against the inode's i_version directly, and is always behind.
We're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime comparison,
if STATX_CHANGE_COOKIE is not supplied in the result mask.
EVM is largely left alone since there's no trivial way to query a file
directly in the LSM call paths to obtain kstat.change_cookie &
kstat.ctime to cache. Thus retains accessing i_version directly.
Regression tests will be added to the Linux Test Project instead of
selftest to help catch future file system changes that may impact
future evaluation of IMA.
I'd like this to be backported to at least 6.18 if possible.
Patches 1 & 2 are preparation patches. Ideally patch 2 is squashed into
3, though not strictly necessary.
Below is a simplified test that demonstrates the issue such that
there are multiple unnecessary measurements occurring for actions on
a file in a stacked TMPFS on XFS, prior to the file moved over to TMPFS:
_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_
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
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
Changes in v5:
- Split into patch series. [Mimi]
- Link to v4: https://lore.kernel.org/r/20260129-xfs-ima-fixup-v4-1-6bb89df7b6a3@cloudflare.com
Changes in v4:
- No functional changes.
- Add Reviewed-by & Fixes tags.
- Link to v3: https://lore.kernel.org/r/20260122-xfs-ima-fixup-v3-1-20335a8aa836@cloudflare.com
Changes in v3:
- Prefer timespec64_to_ns() to leverage attr.version. [Roberto]
- s/TPMFS/TMPFS/ in description.
- Link to v2: https://lore.kernel.org/r/20260120-xfs-ima-fixup-v2-1-f332ead8b043@cloudflare.com
Changes in v2:
- Updated commit description + message to clarify the problem.
- compare struct timespec64 to avoid collision possibility [Roberto].
- Don't check inode_attr_changed() in ima_check_last_writer()
- Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
Changes since RFC:
- Remove calls to I_IS_VERSION()
- Function documentation/comments
- Abide IMA/EVM change detection fallback invariants
- Combined ctime guard into version for attributes struct
- Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
---
Frederick Lawler (3):
ima: Unify vfs_getattr_nosec() stat comparisons under helper function
ima: Make integrity_inode_attrs_changed() call into vfs
ima: Use kstat.ctime as a fallback change detection for stacked fs
include/linux/integrity.h | 43 +++++++++++++++++++++++++++++++++++----
security/integrity/evm/evm_main.c | 5 ++---
security/integrity/ima/ima_api.c | 11 +++++++---
security/integrity/ima/ima_main.c | 11 +++++-----
4 files changed, 54 insertions(+), 16 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/3] ima: Unify vfs_getattr_nosec() stat comparisons under helper function
2026-01-30 22:39 [PATCH v5 0/3] ima: Detect changes to files via kstat changes rather than i_version Frederick Lawler
@ 2026-01-30 22:39 ` Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs Frederick Lawler
2 siblings, 0 replies; 10+ messages in thread
From: Frederick Lawler @ 2026-01-30 22:39 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
The logic for comparing kstat.change_cookie against IMA version is
hard to read. Abstract comparison logic into a new function
integrity_inode_attrs_stat_changed().
No functional change intended.
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
include/linux/integrity.h | 11 +++++++++++
security/integrity/ima/ima_main.c | 4 ++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..beb9ab19fa6257e79266b58bcb5f55b0c5445828 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -49,6 +49,17 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
attrs->ino = inode->i_ino;
}
+/* Compares stat attributes for change detection. */
+static inline bool
+integrity_inode_attrs_stat_changed
+(const struct integrity_inode_attributes *attrs, const struct kstat *stat)
+{
+ if (stat->result_mask & STATX_CHANGE_COOKIE)
+ return stat->change_cookie != attrs->version;
+
+ return true;
+}
+
/*
* On stacked filesystems detect whether the inode or its content has changed.
*/
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..6570ad10887b9ea1172c78274cf62482350e87ff 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -201,8 +201,8 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
vfs_getattr_nosec(&file->f_path, &stat,
STATX_CHANGE_COOKIE,
AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ integrity_inode_attrs_stat_changed(&iint->real_inode,
+ &stat)) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
2026-01-30 22:39 [PATCH v5 0/3] ima: Detect changes to files via kstat changes rather than i_version Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 1/3] ima: Unify vfs_getattr_nosec() stat comparisons under helper function Frederick Lawler
@ 2026-01-30 22:39 ` Frederick Lawler
2026-02-04 12:34 ` Roberto Sassu
2026-01-30 22:39 ` [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs Frederick Lawler
2 siblings, 1 reply; 10+ messages in thread
From: Frederick Lawler @ 2026-01-30 22:39 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
2026-01-30 22:39 [PATCH v5 0/3] ima: Detect changes to files via kstat changes rather than i_version Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 1/3] ima: Unify vfs_getattr_nosec() stat comparisons under helper function Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs Frederick Lawler
@ 2026-01-30 22:39 ` Frederick Lawler
2026-02-04 12:36 ` Roberto Sassu
2 siblings, 1 reply; 10+ messages in thread
From: Frederick Lawler @ 2026-01-30 22:39 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
IMA performs unnecessary measurements on files in stacked file systems
that do not set kstat.change_cookie to an inode's i_version.
For example: TMPFS (upper) is stacked onto XFS (lower).
Actions files result in re-measurement because commit 1cf7e834a6fb
("xfs: switch to multigrain timestamps") introduced multigrain
timestamps into XFS which changed the kstat.change_cookie semantics
to no longer supply an i_version to compare against in
integrity_inode_attributes_changed(). Once the inode is in TMPFS,
the change detection behavior operates as normal because TMPFS updates
kstat.change_cookie to the i_version.
Instead, fall back onto a ctime comparison. This also gives file systems
that do not support i_version an opportunity avoid the same behavior,
as they're more likely to have ctime support.
timespec64_to_ns() is chosen to avoid adding extra storage to
integrity_inode_attributes by leveraging the existing version field.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
include/linux/integrity.h | 6 +++++-
security/integrity/ima/ima_api.c | 11 ++++++++---
security/integrity/ima/ima_main.c | 2 +-
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 382c783f0fa3ae4a938cdf9559291ba1903a378e..ec2c94907f417c4a71ecce29ac79edac9bc2c6f8 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -10,6 +10,7 @@
#include <linux/fs.h>
#include <linux/iversion.h>
#include <linux/kernel.h>
+#include <linux/time64.h>
enum integrity_status {
INTEGRITY_PASS = 0,
@@ -58,6 +59,9 @@ integrity_inode_attrs_stat_changed
if (stat->result_mask & STATX_CHANGE_COOKIE)
return stat->change_cookie != attrs->version;
+ if (stat->result_mask & STATX_CTIME)
+ return timespec64_to_ns(&stat->ctime) != (s64)attrs->version;
+
return true;
}
@@ -84,7 +88,7 @@ integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
* only for IMA if vfs_getattr_nosec() fails.
*/
if (!file || vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT))
return !IS_I_VERSION(inode) ||
!inode_eq_iversion(inode, attrs->version);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ else if (stat.result_mask & STATX_CTIME)
+ i_version = timespec64_to_ns(&stat.ctime);
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8cb17c9d446caaa5a98f5ec8f027c17ba7babca8..776db158b0bd8a0d053729ac0cc15af8b6020a98 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -199,7 +199,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
&iint->atomic_flags);
if ((iint->flags & IMA_NEW_FILE) ||
vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT) ||
integrity_inode_attrs_stat_changed(&iint->real_inode,
&stat)) {
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
2026-01-30 22:39 ` [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs Frederick Lawler
@ 2026-02-04 12:34 ` Roberto Sassu
2026-02-04 19:35 ` Frederick Lawler
0 siblings, 1 reply; 10+ messages in thread
From: Roberto Sassu @ 2026-02-04 12:34 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
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;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
2026-01-30 22:39 ` [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs Frederick Lawler
@ 2026-02-04 12:36 ` Roberto Sassu
2026-02-04 21:22 ` Frederick Lawler
0 siblings, 1 reply; 10+ messages in thread
From: Roberto Sassu @ 2026-02-04 12:36 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> IMA performs unnecessary measurements on files in stacked file systems
> that do not set kstat.change_cookie to an inode's i_version.
>
> For example: TMPFS (upper) is stacked onto XFS (lower).
> Actions files result in re-measurement because commit 1cf7e834a6fb
> ("xfs: switch to multigrain timestamps") introduced multigrain
> timestamps into XFS which changed the kstat.change_cookie semantics
> to no longer supply an i_version to compare against in
> integrity_inode_attributes_changed(). Once the inode is in TMPFS,
> the change detection behavior operates as normal because TMPFS updates
> kstat.change_cookie to the i_version.
>
> Instead, fall back onto a ctime comparison. This also gives file systems
> that do not support i_version an opportunity avoid the same behavior,
> as they're more likely to have ctime support.
>
> timespec64_to_ns() is chosen to avoid adding extra storage to
> integrity_inode_attributes by leveraging the existing version field.
Correct me if I'm wrong, but this issue seems to me xfs-specific. In
that case, maybe it is better to remove the stacked filesystem part,
which might be misleading.
Thanks
Roberto
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> include/linux/integrity.h | 6 +++++-
> security/integrity/ima/ima_api.c | 11 ++++++++---
> security/integrity/ima/ima_main.c | 2 +-
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index 382c783f0fa3ae4a938cdf9559291ba1903a378e..ec2c94907f417c4a71ecce29ac79edac9bc2c6f8 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -10,6 +10,7 @@
> #include <linux/fs.h>
> #include <linux/iversion.h>
> #include <linux/kernel.h>
> +#include <linux/time64.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -58,6 +59,9 @@ integrity_inode_attrs_stat_changed
> if (stat->result_mask & STATX_CHANGE_COOKIE)
> return stat->change_cookie != attrs->version;
>
> + if (stat->result_mask & STATX_CTIME)
> + return timespec64_to_ns(&stat->ctime) != (s64)attrs->version;
> +
> return true;
> }
>
> @@ -84,7 +88,7 @@ integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> * only for IMA if vfs_getattr_nosec() fails.
> */
> if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT))
> return !IS_I_VERSION(inode) ||
> !inode_eq_iversion(inode, attrs->version);
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + else if (stat.result_mask & STATX_CTIME)
> + i_version = timespec64_to_ns(&stat.ctime);
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8cb17c9d446caaa5a98f5ec8f027c17ba7babca8..776db158b0bd8a0d053729ac0cc15af8b6020a98 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -199,7 +199,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT) ||
> integrity_inode_attrs_stat_changed(&iint->real_inode,
> &stat)) {
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
2026-02-04 12:34 ` Roberto Sassu
@ 2026-02-04 19:35 ` Frederick Lawler
0 siblings, 0 replies; 10+ messages in thread
From: Frederick Lawler @ 2026-02-04 19:35 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
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;
> > }
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
2026-02-04 12:36 ` Roberto Sassu
@ 2026-02-04 21:22 ` Frederick Lawler
2026-02-05 4:26 ` Mimi Zohar
0 siblings, 1 reply; 10+ messages in thread
From: Frederick Lawler @ 2026-02-04 21:22 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
On Wed, Feb 04, 2026 at 01:36:09PM +0100, Roberto Sassu wrote:
> On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> > IMA performs unnecessary measurements on files in stacked file systems
> > that do not set kstat.change_cookie to an inode's i_version.
> >
> > For example: TMPFS (upper) is stacked onto XFS (lower).
> > Actions files result in re-measurement because commit 1cf7e834a6fb
> > ("xfs: switch to multigrain timestamps") introduced multigrain
> > timestamps into XFS which changed the kstat.change_cookie semantics
> > to no longer supply an i_version to compare against in
> > integrity_inode_attributes_changed(). Once the inode is in TMPFS,
> > the change detection behavior operates as normal because TMPFS updates
> > kstat.change_cookie to the i_version.
> >
> > Instead, fall back onto a ctime comparison. This also gives file systems
> > that do not support i_version an opportunity avoid the same behavior,
> > as they're more likely to have ctime support.
> >
> > timespec64_to_ns() is chosen to avoid adding extra storage to
> > integrity_inode_attributes by leveraging the existing version field.
>
> Correct me if I'm wrong, but this issue seems to me xfs-specific. In
> that case, maybe it is better to remove the stacked filesystem part,
> which might be misleading.
I'm using XFS because that's a clear example, but as Jeff pointed out in
an earlier email, there's too many file systems to account for. It's
clear from Jeff's prior email[1] that the intention is to stop using
change cookie for change detection for more file systems in favor
of multigrain ctime.
The stacked part is important because AFAIK, if not on stacked file system
the check in process_measurement() line 329 is skipped because
of the last_writer check doing the atomic read on the write count.
That said, I think Mimi pointed out in an email [2] where multi-grain
file systems are impacted regardless of stacked fs or not due to the last
writer check.
I don't recall coming across that in my tests, but perhaps I did that
specific test wrong? To be sure, I created the C program, and on the VM,
created a XFS disk, mounted it on loopback, ran the rdwr program on
"somefile" multiple times, and only got 1 audit log for it, until I
mutated it with touch, and only got 2 hits: original + after mutation
after running the program multiple times.
I'm not sure what's going on there, so I'll look into that a bit more,
but so far the impact is stacked file systems & multigrain ctime AFAIK.
[1]: https://lore.kernel.org/all/75618d9f454aece9a991d74eaf7ae5160828e901.camel@kernel.org/
[2]: https://lore.kernel.org/all/69540ac3aca536db948b6585b7076946a12ebe36.camel@linux.ibm.com/
>
> Thanks
>
> Roberto
>
> > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > include/linux/integrity.h | 6 +++++-
> > security/integrity/ima/ima_api.c | 11 ++++++++---
> > security/integrity/ima/ima_main.c | 2 +-
> > 3 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index 382c783f0fa3ae4a938cdf9559291ba1903a378e..ec2c94907f417c4a71ecce29ac79edac9bc2c6f8 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -10,6 +10,7 @@
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > #include <linux/kernel.h>
> > +#include <linux/time64.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -58,6 +59,9 @@ integrity_inode_attrs_stat_changed
> > if (stat->result_mask & STATX_CHANGE_COOKIE)
> > return stat->change_cookie != attrs->version;
> >
> > + if (stat->result_mask & STATX_CTIME)
> > + return timespec64_to_ns(&stat->ctime) != (s64)attrs->version;
> > +
> > return true;
> > }
> >
> > @@ -84,7 +88,7 @@ integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > * only for IMA if vfs_getattr_nosec() fails.
> > */
> > if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT))
> > return !IS_I_VERSION(inode) ||
> > !inode_eq_iversion(inode, attrs->version);
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + else if (stat.result_mask & STATX_CTIME)
> > + i_version = timespec64_to_ns(&stat.ctime);
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 8cb17c9d446caaa5a98f5ec8f027c17ba7babca8..776db158b0bd8a0d053729ac0cc15af8b6020a98 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -199,7 +199,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > &iint->atomic_flags);
> > if ((iint->flags & IMA_NEW_FILE) ||
> > vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT) ||
> > integrity_inode_attrs_stat_changed(&iint->real_inode,
> > &stat)) {
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
2026-02-04 21:22 ` Frederick Lawler
@ 2026-02-05 4:26 ` Mimi Zohar
2026-02-06 20:06 ` Frederick Lawler
0 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2026-02-05 4:26 UTC (permalink / raw)
To: Frederick Lawler, Roberto Sassu
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Darrick J. Wong, Christian Brauner,
Josef Bacik, Jeff Layton, linux-kernel, linux-integrity,
linux-security-module, kernel-team
On Wed, 2026-02-04 at 15:22 -0600, Frederick Lawler wrote:
> That said, I think Mimi pointed out in an email [2] where multi-grain
> file systems are impacted regardless of stacked fs or not due to the last
> writer check.
>
> I don't recall coming across that in my tests, but perhaps I did that
> specific test wrong? To be sure, I created the C program, and on the VM,
> created a XFS disk, mounted it on loopback, ran the rdwr program on
> "somefile" multiple times, and only got 1 audit log for it, until I
> mutated it with touch, and only got 2 hits: original + after mutation
> after running the program multiple times.
>
> I'm not sure what's going on there, so I'll look into that a bit more,
> but so far the impact is stacked file systems & multigrain ctime AFAIK.
Make sure you're testing without your patch set applied or at least the last
patch.
Mimi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
2026-02-05 4:26 ` Mimi Zohar
@ 2026-02-06 20:06 ` Frederick Lawler
0 siblings, 0 replies; 10+ messages in thread
From: Frederick Lawler @ 2026-02-06 20:06 UTC (permalink / raw)
To: Mimi Zohar
Cc: Roberto Sassu, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
On Wed, Feb 04, 2026 at 11:26:27PM -0500, Mimi Zohar wrote:
> On Wed, 2026-02-04 at 15:22 -0600, Frederick Lawler wrote:
> > That said, I think Mimi pointed out in an email [2] where multi-grain
> > file systems are impacted regardless of stacked fs or not due to the last
> > writer check.
> >
> > I don't recall coming across that in my tests, but perhaps I did that
> > specific test wrong? To be sure, I created the C program, and on the VM,
> > created a XFS disk, mounted it on loopback, ran the rdwr program on
> > "somefile" multiple times, and only got 1 audit log for it, until I
> > mutated it with touch, and only got 2 hits: original + after mutation
> > after running the program multiple times.
> >
> > I'm not sure what's going on there, so I'll look into that a bit more,
> > but so far the impact is stacked file systems & multigrain ctime AFAIK.
>
> Make sure you're testing without your patch set applied or at least the last
> patch.
>
I'm still not sure what went wrong with my test suite, but spinning up a
VM similar to Mimi's, I was able to reproduce the last writer issue. I
used patch 1 as a baseline because that's noop from base 6.19, and used
that function as a convenient trace point.
And, then running the attached example test for stacked FS works as
expected on both patches 1 (multiple log messages) & 3 (just one log
message).
I left out the dump_stack() in integrity_inode_attrs_stat_change()
for readability in these results. For Mimi's test, we just see
calls into ima_check_last_writer(), and for the attached sample
in 0/0, we get the calls straight from process_measurement() for
stacked fs. Running Mimi's and then attached in that order, I get
double the logs because the FILE_CHECK is hitting the last writer,
and we get the process_measurement() for the binary run.
Based on all of this, I can drop the stacked FS wording in the patch
descriptions, and keep it all focused that the change_cookie semantics
have been changed for multigrain file systems.
Results:
PATCH 1/3
[root@localhost ~]# ./mimi.sh
filename: /usr/bin/date-20260206140427
filename: /usr/bin/date-20260206140427
filename: /usr/bin/date-20260206140427
FAIL: Expected 1 audit event, but found 3.
[ 26.033572] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=0 change_cookie=0
[ 26.034372] ima: fred: ima_check_last_writer: must measure file: "/usr/bin/date-20260206140427"
[ 26.037453] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=0 change_cookie=0
[ 26.038425] ima: fred: ima_check_last_writer: must measure file: "/usr/bin/date-20260206140427"
[ 26.039821] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=0 change_cookie=0
[ 26.041383] ima: fred: ima_check_last_writer: must measure file: "/usr/bin/date-20260206140427"
[root@localhost fred-tests]# ./simple-fedora.sh
314572800 bytes (315 MB, 300 MiB) copied, 0.132908 s, 2.4 GB/s
Fri Feb 6 14:56:18 EST 2026
Fri Feb 6 14:56:18 EST 2026
Fri Feb 6 14:56:18 EST 2026
Fri Feb 6 14:56:18 EST 2026
FAIL: Expected 1 audit event, but found 4.
Note: Does not have dmesg output because this patch didn't put the trace
function into integrity_inode_attrs_changed().
PATCH 3/3
[root@localhost ~]# ./mimi.sh
filename: /usr/bin/date-20260206140141
filename: /usr/bin/date-20260206140141
filename: /usr/bin/date-20260206140141
PASS: Found exactly 1 audit event.
[ 17.191235] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=1770404501462431821 change_cookie=0
[ 17.192213] fred: integrity_inode_attrs_stat_changed result_mask=128 (ctime) version=1770404501462431821 ctime=1770404501462431821
[ 17.196325] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=1770404501462431821 change_cookie=0
[ 17.197380] fred: integrity_inode_attrs_stat_changed result_mask=128 (ctime) version=1770404501462431821 ctime=1770404501462431821
[ 17.199750] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=1770404501462431821 change_cookie=0
[ 17.200682] fred: integrity_inode_attrs_stat_changed result_mask=128 (ctime) version=1770404501462431821 ctime=1770404501462431821
[root@localhost fred-tests]# ./simple-fedora.sh
Fri Feb 6 14:53:30 EST 2026
Fri Feb 6 14:53:30 EST 2026
Fri Feb 6 14:53:30 EST 2026
Fri Feb 6 14:53:30 EST 2026
PASS: Found exactly 1 audit event.
[ 23.315358] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=1770407920086616962 change_cookie=0
[ 23.328978] fred: integrity_inode_attrs_stat_changed result_mask=128 (ctime) version=1770407920086616962 ctime=1770407920086616962
[ 23.332122] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=1770407920086616962 change_cookie=0
[ 23.347162] fred: integrity_inode_attrs_stat_changed result_mask=128 (ctime) version=1770407920086616962 ctime=1770407920086616962
[ 23.352931] fred: integrity_inode_attrs_stat_changed result_mask=0 (change cookie) version=1770407920086616962 change_cookie=0
[ 23.368026] fred: integrity_inode_attrs_stat_changed result_mask=128 (ctime) version=1770407920086616962 ctime=1770407920086616962
Note that the in stacked FS case, process_measurement() skipped the check
for attrs changed, and skipped to measuring. Subsequent calls into
process_measurement() hits the integrity_inode_attrs_stat_changed().
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-02-06 20:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-30 22:39 [PATCH v5 0/3] ima: Detect changes to files via kstat changes rather than i_version Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 1/3] ima: Unify vfs_getattr_nosec() stat comparisons under helper function Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs Frederick Lawler
2026-02-04 12:34 ` Roberto Sassu
2026-02-04 19:35 ` Frederick Lawler
2026-01-30 22:39 ` [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs Frederick Lawler
2026-02-04 12:36 ` Roberto Sassu
2026-02-04 21:22 ` Frederick Lawler
2026-02-05 4:26 ` Mimi Zohar
2026-02-06 20:06 ` Frederick Lawler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox