[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jL_uOy+utmRKDVnbHV7boPw-CCo4t99e0mA62oXDEE0EQ@mail.gmail.com>
Date: Wed, 25 Jan 2017 12:05:33 -0800
From: Kees Cook <keescook@...omium.org>
To: Andrei Vagin <avagin@...tuozzo.com>
Cc: Mike Frysinger <vapier@...too.org>,
LKML <linux-kernel@...r.kernel.org>,
Andy Lutomirski <luto@...capital.net>,
Will Drewry <wad@...omium.org>,
Mike Frysinger <vapier@...omium.org>
Subject: Re: seccomp: dump core when using SECCOMP_RET_KILL
On Tue, Jan 24, 2017 at 4:53 PM, Andrei Vagin <avagin@...tuozzo.com> wrote:
> Hi,
>
> One of CRIU tests fails with this patch:
> https://github.com/xemul/criu/blob/master/test/zdtm/static/seccomp_filter_tsync.c
>
> Before this patch only a thread which called a "wrong" syscall is killed.
> Now a whole process is killed if one of threads called a "wrong" syscall.
Oh ew. I wonder what is causing this? In other do_coredump() callers,
they explicitly call do_group_exit(). Hmmm
-Kees
>
> Before this patch only one thread is killed:
>
> 512 seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_TSYNC, {len=4, filter=0x7fffe111fb10} <unfinished ...>
> 484 kill(30, SIG_0) = 0
> 484 write(1, "Wait for zdtm/static/seccomp_filter_tsync(30) to die for 0.100000\n", 66 <unfinished ...>
> 512 <... seccomp resumed> ) = 0
> 512 futex(0x606420, FUTEX_WAKE_PRIVATE, 1 <unfinished ...>
> 484 <... write resumed> ) = 66
> 512 <... futex resumed> ) = 1
> 484 select(0, NULL, NULL, NULL, {tv_sec=0, tv_usec=100000} <unfinished ...>
> 512 futex(0x7f9e894a19d0, FUTEX_WAIT, 32, NULL <unfinished ...>
> 513 <... futex resumed> ) = 0
> 513 futex(0x606420, FUTEX_WAKE_PRIVATE, 1) = 0
> 513 ptrace(PTRACE_TRACEME) = ?
> 513 +++ killed by SIGSYS +++
> 512 <... futex resumed> ) = 0
>
> After this patch a whole process is killed:
>
> 767 seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_TSYNC, {len=4, filter=0x7ffdeedbcd20}) = 0
> 767 futex(0x606420, FUTEX_WAKE_PRIVATE, 1) = 1
> 768 <... futex resumed> ) = 0
> 767 futex(0x7fab05b229d0, FUTEX_WAIT, 32, NULL <unfinished ...>
> 768 futex(0x606420, FUTEX_WAKE_PRIVATE, 1) = 0
> 768 ptrace(PTRACE_TRACEME <unfinished ...>
> 767 <... futex resumed>) = ?
> 768 <... ptrace resumed>) = ?
> 768 +++ killed by SIGSYS (core dumped) +++
> 767 +++ killed by SIGSYS (core dumped) +++
> 766 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) == SIGSYS && WCOREDUMP(s)}], 0, NULL) = 31
> 766 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED, si_pid=31, si_
>
> Steps to reproduce:
> $ git clone git://github.com/xemul/criu
> $ cd criu/test/zdtm/static/
> $ make seccomp_filter_tsync.out
> $ cat seccomp_filter_tsync.out
>
> On Thu, Jan 19, 2017 at 10:28:57PM -0600, Mike Frysinger wrote:
>> From: Mike Frysinger <vapier@...omium.org>
>>
>> The SECCOMP_RET_KILL mode is documented as immediately killing the
>> process as if a SIGSYS had been sent and not caught (similar to a
>> SIGKILL). However, a SIGSYS is documented as triggering a coredump
>> which does not happen today.
>>
>> This has the advantage of being able to more easily debug a process
>> that fails a seccomp filter. Today, most apps need to recompile and
>> change their filter in order to get detailed info out, or manually run
>> things through strace, or enable detailed kernel auditing. Now we get
>> coredumps that fit into existing system-wide crash reporting setups.
>>
>> >From a security pov, this shouldn't be a problem. Unhandled signals
>> can already be sent externally which trigger a coredump independent of
>> the status of the seccomp filter. The act of dumping core itself does
>> not cause change in execution of the program.
>>
>> URL: https://crbug.com/676357
>> Signed-off-by: Mike Frysinger <vapier@...omium.org>
>> Acked-by: Jorge Lucangeli Obes <jorgelo@...omium.org>
>> Acked-by: Kees Cook <keescook@...omium.org>
>> Acked-by: Kees Cook <keescook@...omium.org>
>> ---
>> kernel/seccomp.c | 29 +++++++++++++++++++++--------
>> 1 file changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index f7ce79a46050..f8f88ebcb3ba 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -16,6 +16,7 @@
>> #include <linux/atomic.h>
>> #include <linux/audit.h>
>> #include <linux/compat.h>
>> +#include <linux/coredump.h>
>> #include <linux/sched.h>
>> #include <linux/seccomp.h>
>> #include <linux/slab.h>
>> @@ -486,6 +487,17 @@ void put_seccomp_filter(struct task_struct *tsk)
>> }
>> }
>>
>> +static void seccomp_init_siginfo(siginfo_t *info, int syscall, int reason)
>> +{
>> + memset(info, 0, sizeof(*info));
>> + info->si_signo = SIGSYS;
>> + info->si_code = SYS_SECCOMP;
>> + info->si_call_addr = (void __user *)KSTK_EIP(current);
>> + info->si_errno = reason;
>> + info->si_arch = syscall_get_arch();
>> + info->si_syscall = syscall;
>> +}
>> +
>> /**
>> * seccomp_send_sigsys - signals the task to allow in-process syscall emulation
>> * @syscall: syscall number to send to userland
>> @@ -496,13 +508,7 @@ void put_seccomp_filter(struct task_struct *tsk)
>> static void seccomp_send_sigsys(int syscall, int reason)
>> {
>> struct siginfo info;
>> - memset(&info, 0, sizeof(info));
>> - info.si_signo = SIGSYS;
>> - info.si_code = SYS_SECCOMP;
>> - info.si_call_addr = (void __user *)KSTK_EIP(current);
>> - info.si_errno = reason;
>> - info.si_arch = syscall_get_arch();
>> - info.si_syscall = syscall;
>> + seccomp_init_siginfo(&info, syscall, reason);
>> force_sig_info(SIGSYS, &info, current);
>> }
>> #endif /* CONFIG_SECCOMP_FILTER */
>> @@ -634,10 +640,17 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
>> return 0;
>>
>> case SECCOMP_RET_KILL:
>> - default:
>> + default: {
>> + siginfo_t info;
>> audit_seccomp(this_syscall, SIGSYS, action);
>> + /* Show the original registers in the dump. */
>> + syscall_rollback(current, task_pt_regs(current));
>> + /* Trigger a manual coredump since do_exit skips it. */
>> + seccomp_init_siginfo(&info, this_syscall, data);
>> + do_coredump(&info);
>> do_exit(SIGSYS);
>> }
>> + }
>>
>> unreachable();
>>
--
Kees Cook
Nexus Security
Powered by blists - more mailing lists