linux-kernel - Re: [RFC PATCH 6/7] audit: wake up audit_backlog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1794726.CVITkAglxc@sifl>
Date:	Thu, 05 Nov 2015 20:21:46 -0500
From:	Paul Moore <paul@...l-moore.com>
To:	Richard Guy Briggs <rgb@...hat.com>
Cc:	linux-audit@...hat.com, linux-kernel@...r.kernel.org,
	v.rathor@...il.com
Subject: Re: [RFC PATCH 6/7] audit: wake up audit_backlog_wait queue when auditd goes away.

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 <rgb@...hat.com>
> ---
>  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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/