Re: [BUG][PATCH] audit: audit_log_start running on auditd should not stop

Hi Toshiyuki-san,

On 10/15/2013 12:43 PM, Toshiyuki Okajima wrote: > The backlog cannot be consumed when audit_log_start is running on auditd > even if audit_log_start calls wait_for_auditd to consume it. > The situation is a deadlock because only auditd can consume the backlog. > If the other process needs to send the backlog, it can be also stopped > by the deadlock. > > So, audit_log_start running on auditd should not stop. > > You can see the deadlock with the following reproducer: > # auditctl -a exit,always -S all > # reboot

Hmm, I see, There may be other code paths that auditd can call audit_log_start except audit_log_config_change. so it's better to handle this problem in audit_log_start. but current task is only meaningful when gfp_mask & __GFP_WAIT is true. so maybe the below patch is what you want.

diff --git a/kernel/audit.c b/kernel/audit.c index 7b0e23a..10b4545 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1095,7 +1095,9 @@ struct audit_buffer *audit_log_start(struct audit_context struct audit_buffer *ab = NULL; struct timespec t; unsigned int uninitialized_var(serial); - int reserve; + int reserve = 5; /* Allow atomic callers to go up to five + entries over the normal backlog limit */ + unsigned long timeout_start = jiffies; if (audit_initialized != AUDIT_INITIALIZED) @@ -1104,11 +1106,12 @@ struct audit_buffer *audit_log_start(struct audit_contex if (unlikely(audit_filter_type(type))) return NULL; - if (gfp_mask & __GFP_WAIT) - reserve = 0; - else - reserve = 5; /* Allow atomic callers to go up to five - entries over the normal backlog limit */ + if (gfp_mask & __GFP_WAIT) { + if (audit_pid && audit_pid == current->pid) + gfp_mask &= ~__GFP_WAIT; + else + reserve = 0; + } while (audit_backlog_limit && skb_queue_len(&audit_skb_queue) > audit_backlog_limit + reserv