Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Smtp-Origin-Hostprefix: devvm
From:   Chris Mason <clm@fb.com>
Smtp-Origin-Hostname: devvm005.ftw2.facebook.com
To:     Paul Moore <paul@paul-moore.com>
CC:     Eric Paris <eparis@redhat.com>,
        Dave Jones <davej@codemonkey.org.uk>, <linux-audit@redhat.com>,
        Kyle McMartin <jkkm@fb.com>, <linux-kernel@vger.kernel.org>,
        Chris Mason <clm@fb.com>
Smtp-Origin-Cluster: ftw2c04
Subject: [PATCH] audit: set context->dummy even when audit is off
Date:   Thu, 31 Oct 2019 09:39:31 -0700
Message-ID: <20191031163931.1102669-1-clm@fb.com>
In-Reply-To: <CAHC9VhTyz7fd+iQaymVXUGFe3ZA5Z_WkJeY_snDYiZ9GP6gCOA@mail.gmail.com>
References: <CAHC9VhTyz7fd+iQaymVXUGFe3ZA5Z_WkJeY_snDYiZ9GP6gCOA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Dave Jones reported that we're finding a considerable amount of dmesg
traffic from NTP time adjustments being reported through the audit
subsystem.  His original post is here:

https://lore.kernel.org/lkml/20190923155041.GA14807@codemonkey.org.uk/

The confusing part is that we're seeing this on machines that don't have
audit on.  The NTP code uses audit_dummy_context() to decide if it
should log things:

	static inline void audit_ntp_log(const struct audit_ntp_data *ad)
	{
		if (!audit_dummy_context())
			__audit_ntp_log(ad);
	}

I confirmed with perf probes that:

	context->dummy = 0
	audit_n_rules = 0
	audit_enabled = 0
	audit_ever_enabled = 1 // seems to be from journald

The box boots, journald turns audit on, some time later our
configuration management runs around and turns audit off.  This journald
feature is discussed here: https://github.com/systemd/systemd/issues/959

From what I can tell, audit_syscall_entry is responsible for setting
context->dummy, but we never get down to the test for audit_n_rules:

__audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
                           unsigned long a3, unsigned long a4)
{
        struct audit_context *context = audit_context();
        enum audit_state     state;

        if (!audit_enabled || !context)
                return;
                ^^^^^^^^^^^^^^^^^^  --- we bail here

	[ ... ]

        context->dummy = !audit_n_rules;

This leaves context->dummy at 0, which appears to be the original value
from kzalloc().

If you've gotten this far, you've read everything I know about the audit
code.  With that said, my preference is to make a single source of truth for
decisions about logging.  This commit changes __audit_syscall_entry() to
set context->dummy when audit is off.

Reported-by: Dave Jones <davej@codemonkey.org.uk>
Signed-off-by: Chris Mason <clm@fb.com>
---
 kernel/auditsc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4effe01ebbe2..a5c82d8f9c2b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1631,8 +1631,19 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
 	struct audit_context *context = audit_context();
 	enum audit_state     state;
 
-	if (!audit_enabled || !context)
+	if (!context)
+		return;
+
+	if (!audit_enabled) {
+		/*
+		 * ntp clock adjustments and a few other places check for
+		 * a dummy context without checking to see if audit
+		 * is enabled.  Make sure we set context->dummy when audit
+		 * is off, otherwise they will try to log things.
+		 */
+		context->dummy = 1;
 		return;
+	}
 
 	BUG_ON(context->in_syscall || context->name_count);
 
-- 
2.17.1