Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752570AbbHEJZp (ORCPT ); Wed, 5 Aug 2015 05:25:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58124 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752518AbbHEJZj (ORCPT ); Wed, 5 Aug 2015 05:25:39 -0400 Date: Wed, 5 Aug 2015 05:25:36 -0400 From: Richard Guy Briggs To: Paul Moore Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, sgrubb@redhat.com, eparis@redhat.com Subject: Re: [PATCH V4 (was V6)] audit: save signal match info in case entry passed in is the one deleted Message-ID: <20150805092536.GF32407@madcap2.tricolour.ca> References: <79ae63edcc644461001cd37b8ff1f9b1458f45f7.1438446636.git.rgb@redhat.com> <2901073.DtnZNkzh6Q@sifl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2901073.DtnZNkzh6Q@sifl> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2397 Lines: 64 On 15/08/04, Paul Moore wrote: > On Saturday, August 01, 2015 03:44:01 PM 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 > > --- > > kernel/auditfilter.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > > index 4cb9b44..afb63b3 100644 > > --- a/kernel/auditfilter.c > > +++ b/kernel/auditfilter.c > > @@ -943,6 +943,7 @@ static inline int audit_del_rule(struct audit_entry > > *entry) int ret = 0; > > #ifdef CONFIG_AUDITSYSCALL > > int dont_count = 0; > > + int match_signal = !audit_match_signal(entry); > > > > /* If either of these, don't count towards total */ > > if (entry->rule.listnr == AUDIT_FILTER_USER || > > @@ -972,7 +973,7 @@ static inline int audit_del_rule(struct audit_entry > > *entry) if (!dont_count) > > audit_n_rules--; > > > > - if (!audit_match_signal(entry)) > > + if (match_signal) > > audit_signals--; > > #endif > > mutex_unlock(&audit_filter_mutex); > > Why not simply move this second CONFIG_AUDITSYSCALL above the list_del() > calls? Am I missing something? Good point. That did occur to me at one point when I wasn't in front of the code and promptly forgot once I was. That will neatly remove the temporary variable. > Also, while we're fixing up audit_del_rule(), why not also move the > mutex_unlock() call to after the "out" jump target and then drop the > mutex_unlock() call in the audit_find_rule() error case? Not your fault, but > the code seems silly as-is. Yes, agreed. Another nice catch. These changes will affect "audit by executable" so I'll re-spin that patch set to make it easier to apply. > paul moore - RGB -- Richard Guy Briggs Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- 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/