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 <r...@redhat.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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to