Re: [PATCH V5] audit: save signal match info in case entry passed in is the one deleted

On Wednesday, August 05, 2015 05:23:10 AM Richard Guy Briggs wrote: > Move the access to the entry for audit_match_signal() to the beginning of > the function in case the entry found is the same one passed in. This will > enable it to be used by audit_remove_mark_rule(). > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > > Revision history: > v4 -> v5: > Move mutex_unlock after out label. > Move list_del group after test for signal to remove temp variable. > > --- > This patch was split out from the audit by executable path patch set due to > the potential to use it elsewhere. > > In particular, some questions came up while assessing the potential for code > reuse: > > Why does audit_remove_parent_watches() not call audit_del_rule() for > each entry found? > Is audit_signals not properly decremented? > Is audit_n_rules not properly decremented? > > Why does kill_rules() not call audit_del_rule() for each entry > found? Is audit_signals not properly decremented? > Is audit_n_rules not properly decremented? > > kernel/auditfilter.c | 13 ++++++++----- > 1 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 4cb9b44..1b110fb 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -953,7 +953,6 @@ static inline int audit_del_rule(struct audit_entry > *entry) mutex_lock(&audit_filter_mutex); > e = audit_find_rule(entry, &list); > if (!e) { > - mutex_unlock(&audit_filter_mutex); > ret = -ENOENT; > goto out; > } > @@ -964,9 +963,8 @@ static inline int audit_del_rule(struct audit_entry > *entry) if (e->rule.tree) > audit_remove_tree_rule(&e->rule); > > - list_del_rcu(&e->list); > - list_del(&e->rule.list); > - call_rcu(&e->rcu, audit_free_rule_rcu); > + if (e->rule.exe) > + audit_remove_mark_rule(&e->rule); What?

I think you munged a cut n' paste somehow. This code doesn't even compile.

> #ifdef CONFIG_AUDITSYSCALL > if (!dont_count) > @@ -975,9 +973,14 @@ static inline int audit_del_rule(struct audit_entry > *entry) if (!audit_match_signal(entry)) > audit_signals--; > #endif > - mutex_unlock(&audit_filter_mutex); > + > + list_del_rcu(&e->list); > + list_del(&e->rule.list); > + call_rcu(&e->rcu, audit_free_rule_rcu); > > out: > + mutex_unlock(&audit_filter_mutex); > + > if (tree) > audit_put_tree(tree); /* that's the temporary one */ paul moore