| Message ID | 20220125224645.79319-17-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Namespace IMA with audit support in IMA-ns | expand |
Hi Stefan, On Tue, 2022-01-25 at 17:46 -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > Implement ima_free_policy_rules() that is needed when an ima_namespace > is freed. > > Only reset temp_ima_appraise when using init_ima_ns. Instead of having to walk the policy rules to know if there are any "appraise" rules, the ima_appraise flag is set. For now, only reset temp_ima_appraise flag on failed policy rule updates for init_ima_ns. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > --- > v9: > - Only reset temp_ima_appraise when using init_ima_ns. > --- > security/integrity/ima/ima.h | 1 + > security/integrity/ima/ima_policy.c | 20 +++++++++++++++++++- > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index aea8fb8d2854..8c757223d549 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -329,6 +329,7 @@ void ima_update_policy_flags(struct ima_namespace *ns); > ssize_t ima_parse_add_rule(struct ima_namespace *ns, char *rule); > void ima_delete_rules(struct ima_namespace *ns); > int ima_check_policy(struct ima_namespace *ns); > +void ima_free_policy_rules(struct ima_namespace *ns); > void *ima_policy_start(struct seq_file *m, loff_t *pos); > void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); > void ima_policy_stop(struct seq_file *m, void *v); > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index e8140e73d80b..47f2d1b5d156 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -1880,13 +1880,31 @@ void ima_delete_rules(struct ima_namespace *ns) > { > struct ima_rule_entry *entry, *tmp; > > - temp_ima_appraise = 0; > + if (ns == &init_ima_ns) > + temp_ima_appraise = 0; > + > list_for_each_entry_safe(entry, tmp, &ns->ima_temp_rules, list) { > list_del(&entry->list); > ima_free_rule(entry); > } > } > > +/** > + * ima_free_policy_rules - free all policy rules > + * @ns: IMA namespace that has the policy > + */ > +void ima_free_policy_rules(struct ima_namespace *ns) > +{ > + struct ima_rule_entry *entry, *tmp; > + > + ima_delete_rules(ns); When the IMA policy is being extended, new rules are temporarily added to the ima_temp_rules list. If the entire set of rules being added are valid, they're appended to the tail. There shouldn't be any rules on the ima_temp_rules list unless the policy is currently being extended. Is that possible at this point? > + > + list_for_each_entry_safe(entry, tmp, &ns->ima_policy_rules, list) { > + list_del(&entry->list); > + ima_free_rule(entry); > + } > +} > + > #define __ima_hook_stringify(func, str) (#func), > > const char *const func_tokens[] = { thanks, Mimi
On 1/28/22 09:02, Mimi Zohar wrote: > Hi Stefan, > > On Tue, 2022-01-25 at 17:46 -0500, Stefan Berger wrote: >> From: Stefan Berger <stefanb@linux.ibm.com> >> >> Implement ima_free_policy_rules() that is needed when an ima_namespace >> is freed. >> >> Only reset temp_ima_appraise when using init_ima_ns. > Instead of having to walk the policy rules to know if there are any > "appraise" rules, the ima_appraise flag is set. For now, only reset > temp_ima_appraise flag on failed policy rule updates for init_ima_ns. Ok, I am taking this whole text. > >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> >> --- >> v9: >> - Only reset temp_ima_appraise when using init_ima_ns. >> --- >> security/integrity/ima/ima.h | 1 + >> security/integrity/ima/ima_policy.c | 20 +++++++++++++++++++- >> 2 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h >> index aea8fb8d2854..8c757223d549 100644 >> --- a/security/integrity/ima/ima.h >> +++ b/security/integrity/ima/ima.h >> @@ -329,6 +329,7 @@ void ima_update_policy_flags(struct ima_namespace *ns); >> ssize_t ima_parse_add_rule(struct ima_namespace *ns, char *rule); >> void ima_delete_rules(struct ima_namespace *ns); >> int ima_check_policy(struct ima_namespace *ns); >> +void ima_free_policy_rules(struct ima_namespace *ns); >> void *ima_policy_start(struct seq_file *m, loff_t *pos); >> void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); >> void ima_policy_stop(struct seq_file *m, void *v); >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index e8140e73d80b..47f2d1b5d156 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -1880,13 +1880,31 @@ void ima_delete_rules(struct ima_namespace *ns) >> { >> struct ima_rule_entry *entry, *tmp; >> >> - temp_ima_appraise = 0; >> + if (ns == &init_ima_ns) >> + temp_ima_appraise = 0; >> + >> list_for_each_entry_safe(entry, tmp, &ns->ima_temp_rules, list) { >> list_del(&entry->list); >> ima_free_rule(entry); >> } >> } >> >> +/** >> + * ima_free_policy_rules - free all policy rules >> + * @ns: IMA namespace that has the policy >> + */ >> +void ima_free_policy_rules(struct ima_namespace *ns) >> +{ >> + struct ima_rule_entry *entry, *tmp; >> + >> + ima_delete_rules(ns); > When the IMA policy is being extended, new rules are temporarily added > to the ima_temp_rules list. If the entire set of rules being added are > valid, they're appended to the tail. > > There shouldn't be any rules on the ima_temp_rules list unless the > policy is currently being extended. Is that possible at this point? Actually, no. Nothing can be left. I am removing this call. I wonder whether to split this patch into into two patches? > >> + >> + list_for_each_entry_safe(entry, tmp, &ns->ima_policy_rules, list) { >> + list_del(&entry->list); >> + ima_free_rule(entry); >> + } >> +} >> + >> #define __ima_hook_stringify(func, str) (#func), >> >> const char *const func_tokens[] = { > thanks, > > Mimi >
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index aea8fb8d2854..8c757223d549 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -329,6 +329,7 @@ void ima_update_policy_flags(struct ima_namespace *ns); ssize_t ima_parse_add_rule(struct ima_namespace *ns, char *rule); void ima_delete_rules(struct ima_namespace *ns); int ima_check_policy(struct ima_namespace *ns); +void ima_free_policy_rules(struct ima_namespace *ns); void *ima_policy_start(struct seq_file *m, loff_t *pos); void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); void ima_policy_stop(struct seq_file *m, void *v); diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index e8140e73d80b..47f2d1b5d156 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1880,13 +1880,31 @@ void ima_delete_rules(struct ima_namespace *ns) { struct ima_rule_entry *entry, *tmp; - temp_ima_appraise = 0; + if (ns == &init_ima_ns) + temp_ima_appraise = 0; + list_for_each_entry_safe(entry, tmp, &ns->ima_temp_rules, list) { list_del(&entry->list); ima_free_rule(entry); } } +/** + * ima_free_policy_rules - free all policy rules + * @ns: IMA namespace that has the policy + */ +void ima_free_policy_rules(struct ima_namespace *ns) +{ + struct ima_rule_entry *entry, *tmp; + + ima_delete_rules(ns); + + list_for_each_entry_safe(entry, tmp, &ns->ima_policy_rules, list) { + list_del(&entry->list); + ima_free_rule(entry); + } +} + #define __ima_hook_stringify(func, str) (#func), const char *const func_tokens[] = {