lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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