From mboxrd@z Thu Jan 1 00:00:00 1970 From: Seth Forshee Subject: Re: [PATCH] audit: make sure we don't let the retry queue grow without bounds Date: Mon, 10 Apr 2017 21:39:28 -0500 Message-ID: <20170411023928.GG4689@ubuntu-hedt> References: <149185766728.14543.3818679159914063670.stgit@sifl> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com (ext-mx10.extmail.prod.ext.phx2.redhat.com [10.5.110.39]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 20CEF7C1E1 for ; Tue, 11 Apr 2017 02:39:32 +0000 (UTC) Received: from mail-io0-f173.google.com (mail-io0-f173.google.com [209.85.223.173]) (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 D631361B8E for ; Tue, 11 Apr 2017 02:39:30 +0000 (UTC) Received: by mail-io0-f173.google.com with SMTP id t68so88332411iof.0 for ; Mon, 10 Apr 2017 19:39:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <149185766728.14543.3818679159914063670.stgit@sifl> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Paul Moore Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com 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. Thanks, Seth