From 3a2f5a59a695a73e0cde9a61e0feae5fa730e936 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 10 Jan 2017 12:28:32 -0500 Subject: security,selinux,smack: kill security_task_wait hook As reported by yangshukui, a permission denial from security_task_wait() can lead to a soft lockup in zap_pid_ns_processes() since it only expects sys_wait4() to return 0 or -ECHILD. Further, security_task_wait() can in general lead to zombies; in the absence of some way to automatically reparent a child process upon a denial, the hook is not useful. Remove the security hook and its implementations in SELinux and Smack. Smack already removed its check from its hook. Reported-by: yangshukui Signed-off-by: Stephen Smalley Acked-by: Casey Schaufler Acked-by: Oleg Nesterov Signed-off-by: Paul Moore --- kernel/exit.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) (limited to 'kernel') diff --git a/kernel/exit.c b/kernel/exit.c index 8f14b866f9f6..60f245190571 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -1360,7 +1359,7 @@ static int wait_task_continued(struct wait_opts *wo, struct task_struct *p) * Returns nonzero for a final return, when we have unlocked tasklist_lock. * Returns zero if the search for a child should continue; * then ->notask_error is 0 if @p is an eligible child, - * or another error from security_task_wait(), or still -ECHILD. + * or still -ECHILD. */ static int wait_consider_task(struct wait_opts *wo, int ptrace, struct task_struct *p) @@ -1380,20 +1379,6 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace, if (!ret) return ret; - ret = security_task_wait(p); - if (unlikely(ret < 0)) { - /* - * If we have not yet seen any eligible child, - * then let this error code replace -ECHILD. - * A permission error will give the user a clue - * to look for security policy problems, rather - * than for mysterious wait bugs. - */ - if (wo->notask_error) - wo->notask_error = ret; - return 0; - } - if (unlikely(exit_state == EXIT_TRACE)) { /* * ptrace == 0 means we are the natural parent. In this case @@ -1486,7 +1471,7 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace, * Returns nonzero for a final return, when we have unlocked tasklist_lock. * Returns zero if the search for a child should continue; then * ->notask_error is 0 if there were any eligible children, - * or another error from security_task_wait(), or still -ECHILD. + * or still -ECHILD. */ static int do_wait_thread(struct wait_opts *wo, struct task_struct *tsk) { -- cgit v1.2.3-59-g8ed1b From b25e67161c295c98acda92123b2dd1e7d8642901 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Thu, 19 Jan 2017 22:28:57 -0600 Subject: seccomp: dump core when using SECCOMP_RET_KILL 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 Acked-by: Jorge Lucangeli Obes Acked-by: Kees Cook Signed-off-by: James Morris --- kernel/seccomp.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) (limited to 'kernel') 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 #include #include +#include #include #include #include @@ -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(); -- cgit v1.2.3-59-g8ed1b