On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keescook(a)chromium.org> wrote:
On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj(a)suse.de>
wrote:
> From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001
> From: Tony Jones <tonyj(a)suse.de>
> Date: Sat, 10 Oct 2015 19:30:49 -0700
> Subject: [PATCH] Don't log seccomp messages when audit is disabled
>
> Don't log seccomp messages when audit is disabled.
This is intentional since violation of a seccomp policy ought to
indicate a misbehaving program, and we want these to always be
presented to the system log, regardless of audit being enabled. (I'd
like to even produce system log entries when there is no CONFIG_AUDIT
too, but that's for the future.)
I agree. As I mentioned earlier these AUDIT_SECCOMP records are very handy.
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b2abc99..8f70f3f 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -113,6 +113,12 @@ struct filename;
>
> extern void audit_log_session_info(struct audit_buffer *ab);
>
> +#ifdef CONFIG_AUDIT
> +extern u32 audit_enabled;
> +#else
> +#define audit_enabled 0
> +#endif
> +
> #ifdef CONFIG_AUDIT_COMPAT_GENERIC
> #define audit_is_compat(arch) (!((arch) & __AUDIT_ARCH_64BIT))
> #else
> @@ -213,7 +219,7 @@ void audit_core_dumps(long signr);
> static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> {
> /* Force a record to be reported if a signal was delivered. */
> - if (signr || unlikely(!audit_dummy_context()))
What is dummy_context part of this actually do? I don't think reports
should be made when signr == 0.
The idea behind audit_dummy_context() is to skip auditing when there
are no audit rules configured, it's a performance tweak. My guess is
that Tony's system loads some audit configuration at boot which
enables audit (the kernel starts with audit_enabled=0 ...) and loads a
few syscall filter rules which are enough to make
audit_dummy_context() return false. Can you confirm that Tony?
As for logging seccomp actions when signr == 0, I personally think
that still might be useful as the normal behavior has been altered; I
tend to think any action != ALLOW is worth logging. However, I'm open
to discussion on this if others feel strongly.
> + if (audit_enabled && (signr ||
unlikely(!audit_dummy_context())))
> __audit_seccomp(syscall, signr, code);
> }
--
paul moore
www.paul-moore.com