Re: [PATCH] audit: make sure we don't let the retry queue grow without bounds

[Paul Moore <paul@paul-moore.com>]

On Mon, Apr 10, 2017 at 10:39 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Mon, Apr 10, 2017 at 04:54:27PM -0400, Paul Moore wrote:
>> From: Paul Moore <paul@paul-moore.com>
>>
>> The retry queue is intended to provide a temporary buffer in the case
>> of transient errors when communicating with auditd, it is not meant
>> as a long life queue, that functionality is provided by the hold
>> queue.
>>
>> This patch fixes a problem identified by Seth where the retry queue
>> could grow uncontrollably if an auditd instance did not connect to
>> the kernel to drain the queues.  This commit fixes this by doing the
>> following:
>>
>> * Make sure we always call auditd_reset() if we decide the connection
>> with audit is really dead.  There were some cases in
>> kauditd_hold_skb() where we did not reset the connection, this patch
>> relocates the reset calls to kauditd_thread() so all the error
>> conditions are caught and the connection reset.  As a side effect,
>> this means we could move auditd_reset() and get rid of the forward
>> definition at the top of kernel/audit.c.
>>
>> * We never checked the status of the auditd connection when
>> processing the main audit queue which meant that the retry queue
>> could grow unchecked.  This patch adds a call to auditd_reset()
>> after the main queue has been processed if auditd is not connected,
>> the auditd_reset() call will make sure the retry and hold queues are
>> correctly managed/flushed so that the retry queue remains reasonable.
>>
>> Reported-by: Seth Forshee <seth.forshee@canonical.com>
>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> That fixes the issues I reported. The fix is also needed in 4.10 stable.

Great, thanks for testing.  I'll mark it for stable and send it up to Linus.

-- 
paul moore
www.paul-moore.com