linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ima: ima/lsm policy rule loading logic bug fixes
@ 2020-01-09 14:08 Janne Karhunen
  2020-01-09 14:54 ` Mimi Zohar
  0 siblings, 1 reply; 2+ messages in thread
From: Janne Karhunen @ 2020-01-09 14:08 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, zohar
  Cc: Janne Karhunen, Casey Schaufler, Konsta Karsisto

Keep the ima policy rules around from the beginning even
if they appear invalid at the time of loading, as they
may become active after the lsm policy load. In other
words, now the lsm and the ima can be initialized in any
order and the handling logic is the same as with the lsm
rule reload event.

Patch also fixes the rule re-use during the lsm policy
reload and makes some prints a bit more human readable.

Cc: Casey Schaufler <casey@schaufler-ca.com>
Reported-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>
---
 security/integrity/ima/ima_policy.c | 44 ++++++++++++++---------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a4dde9d575b2..4022c7736fc3 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -265,7 +265,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry)
 static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
 {
 	struct ima_rule_entry *nentry;
-	int i, result;
+	int i;
 
 	nentry = kmalloc(sizeof(*nentry), GFP_KERNEL);
 	if (!nentry)
@@ -279,7 +279,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
 	memset(nentry->lsm, 0, FIELD_SIZEOF(struct ima_rule_entry, lsm));
 
 	for (i = 0; i < MAX_LSM_RULES; i++) {
-		if (!entry->lsm[i].rule)
+		if (!entry->lsm[i].args_p)
 			continue;
 
 		nentry->lsm[i].type = entry->lsm[i].type;
@@ -288,13 +288,13 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
 		if (!nentry->lsm[i].args_p)
 			goto out_err;
 
-		result = security_filter_rule_init(nentry->lsm[i].type,
-						   Audit_equal,
-						   nentry->lsm[i].args_p,
-						   &nentry->lsm[i].rule);
-		if (result == -EINVAL)
-			pr_warn("ima: rule for LSM \'%d\' is undefined\n",
-				entry->lsm[i].type);
+		security_filter_rule_init(nentry->lsm[i].type,
+					  Audit_equal,
+					  nentry->lsm[i].args_p,
+					  &nentry->lsm[i].rule);
+		if (!nentry->lsm[i].rule)
+			pr_warn("rule for LSM \'%s\' is undefined\n",
+				(char *)entry->lsm[i].args_p);
 	}
 	return nentry;
 
@@ -331,7 +331,9 @@ static void ima_lsm_update_rules(void)
 	list_for_each_entry_safe(entry, e, &ima_policy_rules, list) {
 		needs_update = 0;
 		for (i = 0; i < MAX_LSM_RULES; i++) {
-			if (entry->lsm[i].rule) {
+			if (entry->lsm[i].args_p) {
+				pr_info("rule for LSM \'%s\' needs update\n",
+					(char *)entry->lsm[i].args_p);
 				needs_update = 1;
 				break;
 			}
@@ -341,8 +343,7 @@ static void ima_lsm_update_rules(void)
 
 		result = ima_lsm_update_rule(entry);
 		if (result) {
-			pr_err("ima: lsm rule update error %d\n",
-				result);
+			pr_err("lsm rule update error %d\n", result);
 			return;
 		}
 	}
@@ -865,8 +866,6 @@ static const match_table_t policy_tokens = {
 static int ima_lsm_rule_init(struct ima_rule_entry *entry,
 			     substring_t *args, int lsm_rule, int audit_type)
 {
-	int result;
-
 	if (entry->lsm[lsm_rule].rule)
 		return -EINVAL;
 
@@ -875,16 +874,15 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry,
 		return -ENOMEM;
 
 	entry->lsm[lsm_rule].type = audit_type;
-	result = security_filter_rule_init(entry->lsm[lsm_rule].type,
-					   Audit_equal,
-					   entry->lsm[lsm_rule].args_p,
-					   &entry->lsm[lsm_rule].rule);
-	if (!entry->lsm[lsm_rule].rule) {
-		kfree(entry->lsm[lsm_rule].args_p);
-		return -EINVAL;
-	}
+	security_filter_rule_init(entry->lsm[lsm_rule].type,
+				  Audit_equal,
+				  entry->lsm[lsm_rule].args_p,
+				  &entry->lsm[lsm_rule].rule);
+	if (!entry->lsm[lsm_rule].rule)
+		pr_warn("rule for LSM \'%s\' is undefined\n",
+			(char *)entry->lsm[lsm_rule].args_p);
 
-	return result;
+	return 0;
 }
 
 static void ima_log_string_op(struct audit_buffer *ab, char *key, char *value,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ima: ima/lsm policy rule loading logic bug fixes
  2020-01-09 14:08 [PATCH] ima: ima/lsm policy rule loading logic bug fixes Janne Karhunen
@ 2020-01-09 14:54 ` Mimi Zohar
  0 siblings, 0 replies; 2+ messages in thread
From: Mimi Zohar @ 2020-01-09 14:54 UTC (permalink / raw)
  To: Janne Karhunen, linux-integrity, linux-security-module
  Cc: Casey Schaufler, Konsta Karsisto

On Thu, 2020-01-09 at 16:08 +0200, Janne Karhunen wrote:
> Keep the ima policy rules around from the beginning even
> if they appear invalid at the time of loading, as they
> may become active after the lsm policy load. In other
> words, now the lsm and the ima can be initialized in any
> order and the handling logic is the same as with the lsm
> rule reload event.
> 
> Patch also fixes the rule re-use during the lsm policy
> reload and makes some prints a bit more human readable.

Thanks, Janne.  What do you think about adding a single sentence at
the end of this patch description?  Something along the lines of,
"With these changes, there no need to defer loading a custom IMA
policy, based on LSM rules, until after the LSM policy has been
initialized."

The line length, here, is a bit short.  According to section "14) the
canonical path format" of Documentation/process/submitting-
patches.rst, the body of the explanation shouldl be line wrapped at 75
columns.

> 
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Reported-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
> Signed-off-by: Konsta Karsisto <konsta.karsisto@gmail.com>

Please include a "Fixes" tag as well.  Otherwise,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-01-09 14:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 14:08 [PATCH] ima: ima/lsm policy rule loading logic bug fixes Janne Karhunen
2020-01-09 14:54 ` Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).