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

[Paul Moore <paul@paul-moore.com>]

On Thu, Oct 31, 2019 at 12:40 PM Chris Mason <clm@fb.com> 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:
>
> 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(-)

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
www.paul-moore.com