| 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
| ||
|
Message-ID: <aYO4fj0Uw0aUWXOX@CMGLRV3>
Date: Wed, 4 Feb 2026 15:22:06 -0600
From: Frederick Lawler <fred@...udflare.com>
To: Roberto Sassu <roberto.sassu@...weicloud.com>
Cc: 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>, linux-kernel@...r.kernel.org,
linux-integrity@...r.kernel.org,
linux-security-module@...r.kernel.org, kernel-team@...udflare.com
Subject: Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change
detection for stacked fs
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@...nel.org>
> > Signed-off-by: Frederick Lawler <fred@...udflare.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)) {
> >
>
Powered by blists - more mailing lists