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

From: Paul Moore
Date: Thu Oct 31 2019 - 19:27:21 EST

On Thu, Oct 31, 2019 at 12:40 PM Chris Mason <clm@xxxxxx> wrote:
> 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:
> 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:
> 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@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Chris Mason <clm@xxxxxx>
> ---
> kernel/auditsc.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)

Hi Chris,

This is a rather hasty email as I'm at a conference right now, but I
wanted to convey that I'm not opposed to making sure that the NTP
records obey the audit configuration (that was the original intent
after all), I think it is just that we are all a little confused as to
why you are seeing the NTP records *and*only* the NTP records.

It's been a while, but I thought we suggested Dave try running
'auditctl -a never,task' to see if that would solve his problem and I
believe his answer was no, which confused me a bit as the
audit_filter_task() call in audit_alloc() should see that rule and
return a state of AUDIT_DISABLED which not only prevents audit_alloc()
from allocating an audit_context (and remember if the audit_context is
NULL then audit_dummy_context() returns true), but it also clears the
TIF_SYSCALL_AUDIT flag (which I'm guessing you also want).

Can you confirm the results of 'auditctl -a never,task' on your systems?

paul moore