From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Moore Subject: Re: [PATCH] audit: make sure we don't let the retry queue grow without bounds Date: Tue, 11 Apr 2017 10:44:22 -0400 Message-ID: References: <149185766728.14543.3818679159914063670.stgit@sifl> <20170411023928.GG4689@ubuntu-hedt> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2D56B18EEF for ; Tue, 11 Apr 2017 14:44:26 +0000 (UTC) Received: from mail-ua0-f175.google.com (mail-ua0-f175.google.com [209.85.217.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B92C43DBCB for ; Tue, 11 Apr 2017 14:44:23 +0000 (UTC) Received: by mail-ua0-f175.google.com with SMTP id q26so57026924uaa.0 for ; Tue, 11 Apr 2017 07:44:23 -0700 (PDT) In-Reply-To: <20170411023928.GG4689@ubuntu-hedt> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Seth Forshee Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com On Mon, Apr 10, 2017 at 10:39 PM, Seth Forshee wrote: > On Mon, Apr 10, 2017 at 04:54:27PM -0400, Paul Moore wrote: >> From: Paul Moore >> >> 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 >> Signed-off-by: Paul Moore > > 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