lkml.org 
[lkml]   [2016]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH] audit: Don't spam logs with SECCOMP_KILL/RET_ERRNO by default
From
On Mon, Apr 11, 2016 at 12:13 AM, Andi Kleen <andi@firstfloor.org> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> When I run chrome on my opensuse system every time I open
> a new tab the system log is spammed with:
>
> audit[16857]: SECCOMP auid=1000 uid=1000 gid=100 ses=1 pid=16857
> comm="chrome" exe="/opt/google/chrome/chrome" sig=0 arch=c000003e
> syscall=273 compat=0 ip=0x7fe27c11a444 code=0x50000
>
> This happens because chrome uses SECCOMP for its sandbox,
> and for some reason always reaches a SECCOMP_KILL or more likely
> SECCOMP_RET_ERRNO in the rule set.
>
> The seccomp auditing was originally added by Eric with
>
> commit 85e7bac33b8d5edafc4e219c7dfdb3d48e0b4e31
> Author: Eric Paris <eparis@redhat.com>
> Date: Tue Jan 3 14:23:05 2012 -0500
>
> seccomp: audit abnormal end to a process due to seccomp
>
> The audit system likes to collect information about processes that end
> abnormally (SIGSEGV) as this may me useful intrusion detection information.
> This patch adds audit support to collect information when seccomp
> forces a task to exit because of misbehavior in a similar way.
>
> I don't have any other syscall auditing enabled,
> just the standard user space auditing used by the systemd
> and PAM userland. So basic auditing is alwas enabled,
> but no other kernel auditing.
>
> Add a sysctl to enable this unconditional behavior with default
> to off. This replaces an earlier patch that simply checked
> whether syscall auditing was on, but Paul Moore preferred
> this more elaborate approach.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> Documentation/sysctl/kernel.txt | 9 +++++++++
> include/linux/audit.h | 4 +++-
> kernel/seccomp.c | 4 ++++
> kernel/sysctl.c | 11 +++++++++++
> 4 files changed, 27 insertions(+), 1 deletion(-)

Quick response as I'm traveling the next few days and
time/connectivity will be spotty ... thanks for sending an updated
patch, some initial thoughts:

* My thinking was that the sysctl knob could be a threshold value such
that setting it to 0x00030000 would only log TRAP and KILL.
* With the sysctl tunable defaulting to no-logging there is no need to
check for audit_enabled, further, checking for audit_enabled would
prevent logging to dmesg/syslog which I believe is valuable (you may
not).
* A bit nitpicky, but considering the possibility of logging to
dmesg/syslog when auditing is disabled, I think
"seccomp-log-threshold" or similar would be a better sysctl name.

> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 57653a4..abc6ef9 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -21,6 +21,7 @@ show up in /proc/sys/kernel:
> - acct
> - acpi_video_flags
> - auto_msgmni
> +- audit_log_seccomp
> - bootloader_type [ X86 only ]
> - bootloader_version [ X86 only ]
> - callhome [ S390 only ]
> @@ -129,6 +130,14 @@ upon memory add/remove or upon ipc namespace creation/removal.
> Echoing "1" into this file enabled msgmni automatic recomputing.
> Echoing "0" turned it off. auto_msgmni default value was 1.
>
> +==============================================================
> +
> +audit_log_seccomp
> +
> +When this variable is set to 1 every SECCOMP_KILL/SECCOMP_RET_ERRNO
> +results in an audit log. This is generally a bad idea because
> +it leads to a audit message every time Chrome opens a new tab.
> +Defaults to 0.
>
> ==============================================================
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index e38e3fc..c7787ba 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -315,9 +315,11 @@ static inline void audit_inode_child(struct inode *parent,
> }
> void audit_core_dumps(long signr);
>
> +extern int audit_log_seccomp;
> +
> static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> {
> - if (!audit_enabled)
> + if (!audit_enabled || !audit_log_seccomp)
> return;
>
> /* Force a record to be reported if a signal was delivered. */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e1e5a35..09a8b03 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -25,6 +25,10 @@
> #include <asm/syscall.h>
> #endif
>
> +#ifdef CONFIG_AUDIT
> +int audit_log_seccomp __read_mostly = 0;
> +#endif
> +
> #ifdef CONFIG_SECCOMP_FILTER
> #include <linux/filter.h>
> #include <linux/pid.h>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 725587f..0c7611e 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -65,6 +65,7 @@
> #include <linux/sched/sysctl.h>
> #include <linux/kexec.h>
> #include <linux/bpf.h>
> +#include <linux/audit.h>
>
> #include <asm/uaccess.h>
> #include <asm/processor.h>
> @@ -529,6 +530,16 @@ static struct ctl_table kern_table[] = {
> .proc_handler = proc_dointvec,
> },
> #endif
> +#ifdef CONFIG_AUDIT
> + {
> + .procname = "audit-log-seccomp",
> + .data = &audit_log_seccomp,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec,
> + },
> +
> +#endif
> {
> .procname = "print-fatal-signals",
> .data = &print_fatal_signals,
> --
> 2.7.4
>



--
paul moore
www.paul-moore.com

\
 
 \ /
  Last update: 2016-04-11 16:02    [W:0.085 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site