From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031398AbbKFBVu (ORCPT ); Thu, 5 Nov 2015 20:21:50 -0500 Received: from mail-yk0-f182.google.com ([209.85.160.182]:36252 "EHLO mail-yk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030248AbbKFBVs (ORCPT ); Thu, 5 Nov 2015 20:21:48 -0500 From: Paul Moore To: Richard Guy Briggs Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, v.rathor@gmail.com Subject: Re: [RFC PATCH 6/7] audit: wake up audit_backlog_wait queue when auditd goes away. Date: Thu, 05 Nov 2015 20:21:46 -0500 Message-ID: <1794726.CVITkAglxc@sifl> User-Agent: KMail/4.14.10 (Linux/4.2.3-gentoo; KDE/4.14.13; x86_64; ; ) In-Reply-To: <473f52eb9d2c5d218106447084f72f6cb61245de.1445539473.git.rgb@redhat.com> References: <473f52eb9d2c5d218106447084f72f6cb61245de.1445539473.git.rgb@redhat.com> 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 On Thursday, October 22, 2015 02:53:19 PM Richard Guy Briggs wrote: > When auditd goes away (died, killed or shutdown, or net namespace shut > down), there is no point in sleeping waiting for auditd to drain the > queue since that message would be distined for the hold queue after the > timeout anyways. This will needlessly have those processes wait the > full default timeout of 60 seconds (audit_backlog_wait_time). > > Wake up the processes caught in the audit_backlog_wait queue when auditd > is no longer present so they can be sent instead to the hold queue. > > Signed-off-by: Richard Guy Briggs > --- > kernel/audit.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 34411af..688fa1e 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -425,6 +425,7 @@ restart: > audit_log_lost(s); > audit_pid = 0; > audit_sock = NULL; > + wake_up(&audit_backlog_wait); > } else { > pr_warn("re-scheduling(#%d) write to audit_pid=%d\n", > attempts, audit_pid); > @@ -882,6 +883,8 @@ static int audit_receive_msg(struct sk_buff *skb, struct > nlmsghdr *nlh) audit_pid = new_pid; > audit_nlk_portid = NETLINK_CB(skb).portid; > audit_sock = skb->sk; > + if (!audit_pid) > + wake_up(&audit_backlog_wait); > } > if (s.mask & AUDIT_STATUS_RATE_LIMIT) { > err = audit_set_rate_limit(s.rate_limit); I'm thinking it might be time for two small, static helper functions, auditd_register() and auditd_unregister() (or similar, feel free to suggest other names), that set/reset the various auditd state variables and handle the wake_up() call. We're duplicating some code that is starting to get non- trivial. I'd also add a comment about why you are calling wake_up() in the unregister function. > @@ -1154,6 +1157,7 @@ static void __net_exit audit_net_exit(struct net *net) > if (sock == audit_sock) { > audit_pid = 0; > audit_sock = NULL; > + wake_up(&audit_backlog_wait); > } > > RCU_INIT_POINTER(aunet->nlsk, NULL); > @@ -1393,7 +1397,7 @@ struct audit_buffer *audit_log_start(struct > audit_context *ctx, gfp_t gfp_mask, sleep_time = timeout_start + > audit_backlog_wait_time - jiffies; if (sleep_time > 0) { > sleep_time = wait_for_auditd(sleep_time); > - if (sleep_time > 0) > + if (audit_pid && sleep_time > 0) > continue; Perhaps handle this in wait_for_auditd()? Right now this is the only caller, but if we use it elsewhere it seems like we would want the same logic. -- paul moore www.paul-moore.com