| Message ID | 20220201203735.164593-7-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Namespace IMA with audit support in IMA-ns | expand |
On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote Let's update the patch description providing a bit more background info: The archictecture specific policy rules, currently defined for EFI and powerpc, require the kexec kernel image and kernel modules to be validly signed and measured, based on the system's secure boot and/or trusted boot mode and the IMA_ARCH_POLICY Kconfig option being enabled. > Move the arch_policy_entry pointer into ima_namespace. Perhaps include something about namespaces being allowed or not allowed to kexec a new kernel or load kernel modules. thanks, Mimi > > When freeing the memory set the pointer to NULL. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > Acked-by: Christian Brauner <brauner@kernel.org> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
On 2/16/22 11:39, Mimi Zohar wrote: > On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote > > Let's update the patch description providing a bit more background > info: > > The archictecture specific policy rules, currently defined for EFI and > powerpc, require the kexec kernel image and kernel modules to be > validly signed and measured, based on the system's secure boot and/or > trusted boot mode and the IMA_ARCH_POLICY Kconfig option being enabled. > >> Move the arch_policy_entry pointer into ima_namespace. > Perhaps include something about namespaces being allowed or not allowed > to kexec a new kernel or load kernel modules. Namespaces are not allowed to kexec but special-casing the init_ima_ns in the code to handle namespaces differently makes it much harder to read the code. I would avoid special-casing init_ima_ns as much as possible and therefore I have moved the arch_policy_entry into the ima_namespace. Stefan > thanks, > > Mimi >> When freeing the memory set the pointer to NULL. >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> Acked-by: Christian Brauner <brauner@kernel.org> >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
On Wed, 2022-02-16 at 15:48 -0500, Stefan Berger wrote: > On 2/16/22 11:39, Mimi Zohar wrote: > > On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote > > > > Let's update the patch description providing a bit more background > > info: > > > > The archictecture specific policy rules, currently defined for EFI and > > powerpc, require the kexec kernel image and kernel modules to be > > validly signed and measured, based on the system's secure boot and/or > > trusted boot mode and the IMA_ARCH_POLICY Kconfig option being enabled. > > > >> Move the arch_policy_entry pointer into ima_namespace. > > Perhaps include something about namespaces being allowed or not allowed > > to kexec a new kernel or load kernel modules. > > Namespaces are not allowed to kexec but special-casing the init_ima_ns > in the code to handle namespaces differently makes it much harder to > read the code. I would avoid special-casing init_ima_ns as much as > possible and therefore I have moved the arch_policy_entry into the > ima_namespace. Please include this in the patch description, but re-write the last line in the 3rd person, like: To avoid special-casing init_ima_ns, as much as possible, move the arch_policy_entry into the ima_namespace. thanks, Mimi
On 2/16/22 15:56, Mimi Zohar wrote: > On Wed, 2022-02-16 at 15:48 -0500, Stefan Berger wrote: >> On 2/16/22 11:39, Mimi Zohar wrote: >>> On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote >>> >>> Let's update the patch description providing a bit more background >>> info: >>> >>> The archictecture specific policy rules, currently defined for EFI and >>> powerpc, require the kexec kernel image and kernel modules to be >>> validly signed and measured, based on the system's secure boot and/or >>> trusted boot mode and the IMA_ARCH_POLICY Kconfig option being enabled. >>> >>>> Move the arch_policy_entry pointer into ima_namespace. >>> Perhaps include something about namespaces being allowed or not allowed >>> to kexec a new kernel or load kernel modules. >> Namespaces are not allowed to kexec but special-casing the init_ima_ns >> in the code to handle namespaces differently makes it much harder to >> read the code. I would avoid special-casing init_ima_ns as much as >> possible and therefore I have moved the arch_policy_entry into the >> ima_namespace. > Please include this in the patch description, but re-write the last > line in the 3rd person, like: > > To avoid special-casing init_ima_ns, as much as possible, move the > arch_policy_entry into the ima_namespace. I took the paragraph on the background as well as this sentence. > > thanks, > > Mimi > >
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);