diff mbox series

ima: Reset IMA_NONACTION_RULE_FLAGS after post_setattr

Message ID 20250204125720.1326257-1-roberto.sassu@huaweicloud.com (mailing list archive)
State New
Headers show
Series ima: Reset IMA_NONACTION_RULE_FLAGS after post_setattr | expand

Commit Message

Roberto Sassu Feb. 4, 2025, 12:57 p.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

Commit 0d73a55208e9 ("ima: re-introduce own integrity cache lock")
mistakenly reverted the performance improvement introduced in commit
42a4c603198f0 ("ima: fix ima_inode_post_setattr"). The unused bit mask was
subsequently removed by commit 11c60f23ed13 ("integrity: Remove unused
macro IMA_ACTION_RULE_FLAGS").

Restore the performance improvement by introducing the new mask
IMA_NONACTION_RULE_FLAGS, equal to IMA_NONACTION_FLAGS without
IMA_NEW_FILE, which is not a rule-specific flag.

Finally, reset IMA_NONACTION_RULE_FLAGS instead of IMA_NONACTION_FLAGS in
process_measurement(), if the IMA_CHANGE_ATTR atomic flag is set (after
file metadata modification).

With this patch, new files for which metadata were modified while they are
still open, can be reopened before the last file close (when security.ima
is written), since the IMA_NEW_FILE flag is not cleared anymore. Otherwise,
appraisal fails because security.ima is missing (files with IMA_NEW_FILE
set are an exception).

Cc: stable@vger.kernel.org # v4.16.x
Fixes: 0d73a55208e9 ("ima: re-introduce own integrity cache lock")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h      | 3 +++
 security/integrity/ima/ima_main.c | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Comments

Mimi Zohar Feb. 5, 2025, 2:59 a.m. UTC | #1 
On Tue, 2025-02-04 at 13:57 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Commit 0d73a55208e9 ("ima: re-introduce own integrity cache lock")
> mistakenly reverted the performance improvement introduced in commit
> 42a4c603198f0 ("ima: fix ima_inode_post_setattr"). The unused bit mask was
> subsequently removed by commit 11c60f23ed13 ("integrity: Remove unused
> macro IMA_ACTION_RULE_FLAGS").
> 
> Restore the performance improvement by introducing the new mask
> IMA_NONACTION_RULE_FLAGS, equal to IMA_NONACTION_FLAGS without
> IMA_NEW_FILE, which is not a rule-specific flag.
> 
> Finally, reset IMA_NONACTION_RULE_FLAGS instead of IMA_NONACTION_FLAGS in
> process_measurement(), if the IMA_CHANGE_ATTR atomic flag is set (after
> file metadata modification).
> 
> With this patch, new files for which metadata were modified while they are
> still open, can be reopened before the last file close (when security.ima
> is written), since the IMA_NEW_FILE flag is not cleared anymore. Otherwise,
> appraisal fails because security.ima is missing (files with IMA_NEW_FILE
> set are an exception).
> 
> Cc: stable@vger.kernel.org # v4.16.x
> Fixes: 0d73a55208e9 ("ima: re-introduce own integrity cache lock")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Thanks!

Mimi
diff mbox series

Patch

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 24d09ea91b87..a4f284bd846c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -149,6 +149,9 @@  struct ima_kexec_hdr {
 #define IMA_CHECK_BLACKLIST	0x40000000
 #define IMA_VERITY_REQUIRED	0x80000000
 
+/* Exclude non-action flags which are not rule-specific. */
+#define IMA_NONACTION_RULE_FLAGS	(IMA_NONACTION_FLAGS & ~IMA_NEW_FILE)
+
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
 #define IMA_DONE_MASK		(IMA_MEASURED | IMA_APPRAISED | IMA_AUDITED | \
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 9b87556b03a7..b028c501949c 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -269,10 +269,13 @@  static int process_measurement(struct file *file, const struct cred *cred,
 	mutex_lock(&iint->mutex);
 
 	if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags))
-		/* reset appraisal flags if ima_inode_post_setattr was called */
+		/*
+		 * Reset appraisal flags (action and non-action rule-specific)
+		 * if ima_inode_post_setattr was called.
+		 */
 		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
 				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
-				 IMA_NONACTION_FLAGS);
+				 IMA_NONACTION_RULE_FLAGS);
 
 	/*
 	 * Re-evaulate the file if either the xattr has changed or the