Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752398AbdDIPnl (ORCPT ); Sun, 9 Apr 2017 11:43:41 -0400 Received: from mail-ua0-f180.google.com ([209.85.217.180]:36368 "EHLO mail-ua0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752203AbdDIPni (ORCPT ); Sun, 9 Apr 2017 11:43:38 -0400 MIME-Version: 1.0 X-Originating-IP: [108.49.102.27] In-Reply-To: <20170409144009.GA44106@ubuntu-hedt> References: <20170409030220.GA33027@ubuntu-hedt> <20170409144009.GA44106@ubuntu-hedt> From: Paul Moore Date: Sun, 9 Apr 2017 11:43:36 -0400 Message-ID: Subject: Re: audit regressions in 4.11 To: Seth Forshee Cc: Eric Paris , linux-audit@redhat.com, linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2486 Lines: 52 On Sun, Apr 9, 2017 at 10:40 AM, Seth Forshee wrote: > On Sun, Apr 09, 2017 at 09:14:03AM -0400, Paul Moore wrote: >> On Sat, Apr 8, 2017 at 11:02 PM, Seth Forshee >> wrote: >> > I've observed audit regressions in 4.11-rc when not using a userspace >> > audit daemon. The most obvious issue is that audit messages are not >> > appearing in dmesg anymore. If a sufficient number of audit messages are >> > generated the kernel will also start invoking the OOM killer. >> > >> > It looks like previously, when there's no auditd in userspace kauditd >> > would call kauditd_hold_skb(), which prints the message using printk and >> > either frees the skb or queues it (with a limit on the number of queued >> > skb's by default). >> > >> > Since 5b52330bbfe6 "audit: fix auditd/kernel connection state tracking" >> > when there's no auditd kauditd will instead use the retry queue, which >> > has no limit. But it will not process the retry queue when there's no >> > auditd, so messages pile up there indefinitely. >> >> Hi Seth, >> >> Thanks for the report. Let me play with this and think on it for a >> bit, but looking at the code again I think the issue is that we check >> to see if auditd is connected at the top of the kauditd_thread() loop >> and if it isn't we skip right to the main_queue label and bypass the >> hold/retry queue processing which has the logic to ensure the retry >> queue is managed correctly. My initial thinking is that the fix is to >> check and see if auditd is connected in kauditd_retry_skb(), if it >> isn't we skip the retry queue and call kauditd_hold_skb(), if auditd >> is connected we add the record to the retry queue (what we currently >> do). > > Yeah, my first thought was to make this change: > > kauditd_send_queue(sk, portid, &audit_queue, 1, > kauditd_send_multicast_skb, > - kauditd_retry_skb); > + sk ? kauditd_retry_skb : kauditd_hold_skb); > > However some scenarios could result in unbounded queueing on the hold > queue as well, so I'm not sure if that's quite enough. At the moment I think I'd prefer to put the auditd check inside kauditd_retry_skb() itself, but you've got the basic idea. Keep in mind that kauditd_hold_skb() already has the logic inside itself to prevent the hold queue from growing out of control. -- paul moore www.paul-moore.com