lkml.org 
[lkml]   [2016]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] seccomp: Fix tracer exit notifications during fatal signals
On Wed, Aug 10, 2016 at 4:37 PM, Kees Cook <keescook@chromium.org> wrote:
> This fixes a ptrace vs fatal pending signals bug as manifested in seccomp
> now that ptrace was reordered to happen after ptrace. The short version is
> that seccomp should not attempt to call do_exit() while fatal signals are
> pending under a tracer. This was needlessly paranoid. Instead, the syscall
> can just be skipped and normal signal handling, tracer notification, and
> process death can happen.
>
> Slightly edited original bug report:
>
> If a tracee task is in a PTRACE_EVENT_SECCOMP trap, or has been resumed
> after such a trap but not yet been scheduled, and another task in the
> thread-group calls exit_group(), then the tracee task exits without the
> ptracer receiving a PTRACE_EVENT_EXIT notification. Test case here:
> https://gist.github.com/khuey/3c43ac247c72cef8c956ca73281c9be7
>
> The bug happens because when __seccomp_filter() detects
> fatal_signal_pending(), it calls do_exit() without dequeuing the fatal
> signal. When do_exit() sends the PTRACE_EVENT_EXIT notification and
> that task is descheduled, __schedule() notices that there is a fatal
> signal pending and changes its state from TASK_TRACED to TASK_RUNNING.
> That prevents the ptracer's waitpid() from returning the ptrace event.
> A more detailed analysis is here:
> https://github.com/mozilla/rr/issues/1762#issuecomment-237396255.
>
> Reported-by: Robert O'Callahan <robert@ocallahan.org>
> Reported-by: Kyle Huey <khuey@kylehuey.com>
> Fixes: 93e35efb8de4 ("x86/ptrace: run seccomp after ptrace")
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> kernel/seccomp.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index ef6c6c3f9d8a..0db7c8a2afe2 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -605,12 +605,16 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
> ptrace_event(PTRACE_EVENT_SECCOMP, data);
> /*
> * The delivery of a fatal signal during event
> - * notification may silently skip tracer notification.
> - * Terminating the task now avoids executing a system
> - * call that may not be intended.
> + * notification may silently skip tracer notification,
> + * which could leave us with a potentially unmodified
> + * syscall that the tracer would have liked to have
> + * changed. Since the process is about to die, we just
> + * force the syscall to be skipped and let the signal
> + * kill the process and correctly handle any tracer exit
> + * notifications.
> */
> if (fatal_signal_pending(current))
> - do_exit(SIGSYS);
> + goto skip;
> /* Check if the tracer forced the syscall to be skipped. */
> this_syscall = syscall_get_nr(current, task_pt_regs(current));
> if (this_syscall < 0)
> --
> 2.7.4
>
>
> --
> Kees Cook
> Nexus Security

This patch also fixes rr as well.

- Kyle

\
 
 \ /
  Last update: 2016-08-11 03:01    [W:0.508 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site