[PATCH v2 3/7] ima: Ensure lock is held when setting iint pointer in inode security blob
From: Roberto Sassu <hidden>
Date: 2024-11-28 10:07:25
Also in:
linux-fsdevel, linux-integrity, lkml
Subsystem:
extended verification module (evm), integrity measurement architecture (ima), security subsystem, the rest · Maintainers:
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Paul Moore, James Morris, "Serge E. Hallyn", Linus Torvalds
From: Roberto Sassu <roberto.sassu@huawei.com> IMA stores a pointer of the ima_iint_cache structure, containing integrity metadata, in the inode security blob. However, check and assignment of this pointer is not atomic, and it might happen that two tasks both see that the iint pointer is NULL and try to set it, causing a memory leak. Ensure that the iint check and assignment is guarded, by adding a lockdep assertion in ima_inode_get(). Consequently, guard the remaining ima_inode_get() calls, in ima_post_create_tmpfile() and ima_post_path_mknod(), to avoid the lockdep warnings. Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/integrity/ima/ima_iint.c | 2 ++ security/integrity/ima/ima_main.c | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index dcc32483d29f..fca9db293c79 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c@@ -97,6 +97,8 @@ struct ima_iint_cache *ima_inode_get(struct inode *inode) if (!iint_lock) return NULL; + lockdep_assert_held(&iint_lock->mutex); + iint = iint_lock->iint; if (iint) return iint;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 05cfb04cd02b..1e474ff6a777 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c@@ -705,14 +705,19 @@ static void ima_post_create_tmpfile(struct mnt_idmap *idmap, if (!must_appraise) return; + ima_iint_lock(inode); + /* Nothing to do if we can't allocate memory */ iint = ima_inode_get(inode); - if (!iint) + if (!iint) { + ima_iint_unlock(inode); return; + } /* needed for writing the security xattrs */ set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); iint->ima_file_status = INTEGRITY_PASS; + ima_iint_unlock(inode); } /**
@@ -737,13 +742,18 @@ static void ima_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry) if (!must_appraise) return; + ima_iint_lock(inode); + /* Nothing to do if we can't allocate memory */ iint = ima_inode_get(inode); - if (!iint) + if (!iint) { + ima_iint_unlock(inode); return; + } /* needed for re-opening empty files */ iint->flags |= IMA_NEW_FILE; + ima_iint_unlock(inode); } /**
--
2.47.0.118.gfd3785337b