linux-kernel - [PATCH] audit: set context->dummy even when audit is off

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191031163931.1102669-1-clm@fb.com>
Date:   Thu, 31 Oct 2019 09:39:31 -0700
From:   Chris Mason <clm@...com>
To:     Paul Moore <paul@...l-moore.com>
CC:     Eric Paris <eparis@...hat.com>,
        Dave Jones <davej@...emonkey.org.uk>, <linux-audit@...hat.com>,
        Kyle McMartin <jkkm@...com>, <linux-kernel@...r.kernel.org>,
        Chris Mason <clm@...com>
Subject: [PATCH] audit: set context->dummy even when audit is off

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@...emonkey.org.uk>
Signed-off-by: Chris Mason <clm@...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