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 <[email protected]> Signed-off-by: Frederick Lawler <[email protected]> --- 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 <[email protected]>
