| 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: <20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com>
Date: Mon, 29 Dec 2025 11:52:19 -0600
From: Frederick Lawler <fred@...udflare.com>
To: 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,
Frederick Lawler <fred@...udflare.com>
Subject: [PATCH RFC] ima: Fallback to a ctime guard without i_version
updates
Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
is no longer able to correctly track inode.i_version due to the struct
kstat.change_cookie no longer containing an updated i_version.
Introduce a fallback mechanism for IMA that instead tracks a
integrity_ctime_guard() in absence of or outdated i_version
for stacked file systems.
EVM is left alone since it mostly cares about the backing inode.
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>
---
The motivation behind this was that file systems that use the
cookie to set the i_version for stacked file systems may still do so.
Then add in the ctime_guard as a fallback if there's a detected change.
The assumption is that the ctime will be different if the i_version is
different anyway for non-stacked file systems.
I'm not too pleased with passing in struct file* to
integrity_inode_attrs_changed() since EVM doesn't currently use
that for now, but I couldn't come up with another idea to get the
stat without coming up with a new stat function to accommodate just
the file path, fully separate out IMA/EVM checks, or lastly add stacked
file system support to EVM (which doesn't make much sense to me
at the moment).
I plan on adding in self test infrastructure for the v1, but I would
like to get some early feedback on the approach first.
---
include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
security/integrity/evm/evm_crypto.c | 2 +-
security/integrity/evm/evm_main.c | 2 +-
security/integrity/ima/ima_api.c | 21 +++++++++++++++------
security/integrity/ima/ima_main.c | 17 ++++++++++-------
5 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
/* An inode's attributes for detection of changes */
struct integrity_inode_attributes {
+ u64 ctime_guard;
u64 version; /* track inode changes */
unsigned long ino;
dev_t dev;
};
+static inline u64 integrity_ctime_guard(struct kstat stat)
+{
+ return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
+}
+
/*
* On stacked filesystems the i_version alone is not enough to detect file data
* or metadata change. Additional metadata is required.
*/
static inline void
integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
- u64 i_version, const struct inode *inode)
+ u64 i_version, u64 ctime_guard,
+ const struct inode *inode)
{
+ attrs->ctime_guard = ctime_guard;
attrs->version = i_version;
attrs->dev = inode->i_sb->s_dev;
attrs->ino = inode->i_ino;
@@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
*/
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;
+
+ if (inode->i_sb->s_dev != attrs->dev ||
+ inode->i_ino != attrs->ino)
+ return true;
+
+ if (inode_eq_iversion(inode, attrs->version))
+ return false;
+
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return true;
+
+ return attrs->ctime_guard != integrity_ctime_guard(stat);
}
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (IS_I_VERSION(inode))
i_version = inode_query_iversion(inode);
integrity_inode_attrs_store(&iint->metadata_inode, i_version,
- inode);
+ 0, inode);
}
/* Portable EVM signatures must include an IMA hash */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
if (iint) {
ret = (!IS_I_VERSION(metadata_inode) ||
integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ NULL, metadata_inode));
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
int length;
void *tmpbuf;
u64 i_version = 0;
+ u64 ctime_guard = 0;
/*
* Always collect the modsig, because IMA might have already collected
@@ -272,10 +273,16 @@ 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;
+
+ if (stat.result_mask & STATX_CTIME)
+ ctime_guard = integrity_ctime_guard(stat);
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
@@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
iint->ima_hash = tmpbuf;
memcpy(iint->ima_hash, &hash, length);
- if (real_inode == inode)
+ if (real_inode == inode) {
iint->real_inode.version = i_version;
- else
+ iint->real_inode.ctime_guard = ctime_guard;
+ } else {
integrity_inode_attrs_store(&iint->real_inode, i_version,
- real_inode);
+ ctime_guard, real_inode);
+ }
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -22,6 +22,7 @@
#include <linux/mount.h>
#include <linux/mman.h>
#include <linux/slab.h>
+#include <linux/stat.h>
#include <linux/xattr.h>
#include <linux/ima.h>
#include <linux/fs.h>
@@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
{
fmode_t mode = file->f_mode;
bool update;
+ int ret;
if (!(mode & FMODE_WRITE))
return;
@@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
update = test_and_clear_bit(IMA_UPDATE_XATTR,
&iint->atomic_flags);
- if ((iint->flags & IMA_NEW_FILE) ||
- 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) {
+ ret = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT);
+ if ((iint->flags & IMA_NEW_FILE) || ret ||
+ (!ret && stat.change_cookie != iint->real_inode.version) ||
+ (!ret && integrity_ctime_guard(stat) !=
+ iint->real_inode.ctime_guard)) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
if (!IS_I_VERSION(real_inode) ||
integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@...udflare.com>
Powered by blists - more mailing lists