Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752743AbbHEPMR (ORCPT ); Wed, 5 Aug 2015 11:12:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58447 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752544AbbHEPMP (ORCPT ); Wed, 5 Aug 2015 11:12:15 -0400 From: Paul Moore To: Richard Guy Briggs Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, sgrubb@redhat.com, eparis@redhat.com Subject: Re: [PATCH V5] audit: save signal match info in case entry passed in is the one deleted Date: Wed, 05 Aug 2015 11:12:13 -0400 Message-ID: <1583724.gJuObs13E5@sifl> Organization: Red Hat User-Agent: KMail/4.14.10 (Linux/4.1.2-gentoo; KDE/4.14.10; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2743 Lines: 84 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 > > 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 security @ redhat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/