thank for the testcase. i'll take a look. when i went through the code visually, i didn't think it was killing, just suspending+resuming for the sake of snapshotting, but i must have misread. -mike
On Tue, Jan 24, 2017 at 2:53 PM, Andrei Vagin <ava...@virtuozzo.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. > > 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 <vap...@chromium.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 <vap...@chromium.org> >> Acked-by: Jorge Lucangeli Obes <jorg...@chromium.org> >> Acked-by: Kees Cook <keesc...@chromium.org> >> Acked-by: Kees Cook <keesc...@chromium.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(); >>
