diff mbox series

[v9,06/23] ima: Move arch_policy_entry into ima_namespace

Message ID 20220125224645.79319-7-stefanb@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show
Series ima: Namespace IMA with audit support in IMA-ns | expand

Commit Message

Stefan Berger Jan. 25, 2022, 10:46 p.m. UTC 
From: Stefan Berger <stefanb@linux.ibm.com>

Move the arch_policy_entry pointer into ima_namespace.

When freeing the memory set the pointer to NULL.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 security/integrity/ima/ima.h             |  3 +++
 security/integrity/ima/ima_init_ima_ns.c |  1 +
 security/integrity/ima/ima_policy.c      | 23 +++++++++++------------
 3 files changed, 15 insertions(+), 12 deletions(-)

Comments

Christian Brauner Jan. 26, 2022, 9:11 a.m. UTC | #1 
On Tue, Jan 25, 2022 at 05:46:28PM -0500, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Move the arch_policy_entry pointer into ima_namespace.
> 
> When freeing the memory set the pointer to NULL.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---

Only relevant for the initial imans (for now) since it is derived from a
boot parameter. Maybe mention this in the commit message.

Move into struct ima_namespace looks good,
Acked-by: Christian Brauner <brauner@kernel.org>
Mimi Zohar Jan. 27, 2022, 7:42 p.m. UTC | #2 
On Wed, 2022-01-26 at 10:11 +0100, Christian Brauner wrote:
> On Tue, Jan 25, 2022 at 05:46:28PM -0500, Stefan Berger wrote:
> > From: Stefan Berger <stefanb@linux.ibm.com>
> > 
> > Move the arch_policy_entry pointer into ima_namespace.
> > 
> > When freeing the memory set the pointer to NULL.
> > 
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> 
> Only relevant for the initial imans (for now) since it is derived from a
> boot parameter. Maybe mention this in the commit message.

Enabling architecture specific policy rules is based on
CONFIG_IMA_ARCH_POLICY.  As the name implies, each architecture is free
to define their own policy rules.  For example on x86, based on the
secure boot mode both measurement and signature verification rules are
defined for the kexec kernel image and kernel modules. Similarly on
powerpc, different measurement and signature verification rules for the
kexec kernel image and kernel modules are defined based on whether
trusted boot, secure boot, or both are enabled [2].

As neither kexec nor loading kernel modules are applicable, the
architecture policy rules are limited to initial imans.

[1] security/integrity/ima/ima_efi.c 
[2] arch/powerpc/kernel/ima_arch.c

> 
> Move into struct ima_namespace looks good,
> Acked-by: Christian Brauner <brauner@kernel.org>

Thanks, Christian.

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

Patch

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 5b44fa6f27c4..a4669b55c2e0 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -125,6 +125,9 @@  struct ima_namespace {
 
 	struct list_head __rcu *ima_rules;	/* current policy */
 	int ima_policy_flag;
+
+	/* An array of architecture specific rules */
+	struct ima_rule_entry *arch_policy_entry;
 } __randomize_layout;
 extern struct ima_namespace init_ima_ns;
 
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
index c919a456b525..ae33621c3955 100644
--- a/security/integrity/ima/ima_init_ima_ns.c
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -15,6 +15,7 @@  static int ima_init_namespace(struct ima_namespace *ns)
 	INIT_LIST_HEAD(&ns->ima_temp_rules);
 	ns->ima_rules = (struct list_head __rcu *)(&ns->ima_default_rules);
 	ns->ima_policy_flag = 0;
+	ns->arch_policy_entry = NULL;
 
 	return 0;
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index b0e1c16b7f37..05b2bc06ab0c 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -229,9 +229,6 @@  static struct ima_rule_entry critical_data_rules[] __ro_after_init = {
 	{.action = MEASURE, .func = CRITICAL_DATA, .flags = IMA_FUNC},
 };
 
-/* An array of architecture specific rules */
-static struct ima_rule_entry *arch_policy_entry __ro_after_init;
-
 static int ima_policy __initdata;
 
 static int __init default_measure_policy_setup(char *str)
@@ -860,9 +857,10 @@  static int __init ima_init_arch_policy(struct ima_namespace *ns)
 	for (rules = arch_rules; *rules != NULL; rules++)
 		arch_entries++;
 
-	arch_policy_entry = kcalloc(arch_entries + 1,
-				    sizeof(*arch_policy_entry), GFP_KERNEL);
-	if (!arch_policy_entry)
+	ns->arch_policy_entry = kcalloc(arch_entries + 1,
+					sizeof(*ns->arch_policy_entry),
+					GFP_KERNEL);
+	if (!ns->arch_policy_entry)
 		return 0;
 
 	/* Convert each policy string rules to struct ima_rule_entry format */
@@ -872,13 +870,13 @@  static int __init ima_init_arch_policy(struct ima_namespace *ns)
 
 		result = strscpy(rule, *rules, sizeof(rule));
 
-		INIT_LIST_HEAD(&arch_policy_entry[i].list);
-		result = ima_parse_rule(ns, rule, &arch_policy_entry[i]);
+		INIT_LIST_HEAD(&ns->arch_policy_entry[i].list);
+		result = ima_parse_rule(ns, rule, &ns->arch_policy_entry[i]);
 		if (result) {
 			pr_warn("Skipping unknown architecture policy rule: %s\n",
 				rule);
-			memset(&arch_policy_entry[i], 0,
-			       sizeof(*arch_policy_entry));
+			memset(&ns->arch_policy_entry[i], 0,
+			       sizeof(ns->arch_policy_entry[i]));
 			continue;
 		}
 		i++;
@@ -926,7 +924,7 @@  void __init ima_init_policy(struct ima_namespace *ns)
 	if (!arch_entries)
 		pr_info("No architecture policies found\n");
 	else
-		add_rules(ns, arch_policy_entry, arch_entries,
+		add_rules(ns, ns->arch_policy_entry, arch_entries,
 			  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
 
 	/*
@@ -1006,7 +1004,8 @@  void ima_update_policy(struct ima_namespace *ns)
 		 * on boot.  After loading a custom policy, free the
 		 * architecture specific rules stored as an array.
 		 */
-		kfree(arch_policy_entry);
+		kfree(ns->arch_policy_entry);
+		ns->arch_policy_entry = NULL;
 	}
 	ima_update_policy_flags(ns);