On Mon, Aug 22, 2016 at 8:27 PM, Kyle Huey <[email protected]> wrote: > On Thu, Aug 11, 2016 at 11:18 AM, Kees Cook <[email protected]> wrote: >> On Thu, Aug 11, 2016 at 8:12 AM, Oleg Nesterov <[email protected]> wrote: >>> On 08/10, Kees Cook 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. >>> >>> ACK. >>> >>> I think this change is fine in any case, but... >>> >>>> 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 >>> >>> I _never_ understood what PTRACE_EVENT_EXIT should actually do. I mean, >>> when it should actually stop. This was never defined. >> >> Yeah, agreed. I spent some time reading through what should happen to >> __TASK_TRACED during exit and my head spun. :) >> >>>> 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. >>> >>> And this can happen anyway, with or without this change, with or without >>> seccomp. Because another fatal signal can be pending. So PTRACE_EVENT_EXIT >>> actually depends on /dev/random. >>> >>> Perhaps we should finally define what it should do. Say, it should only >>> stop if SIGKILL was sent "implicitely" by exit/exec. But as for exec, >>> there are more (off-topic) complications, not sure we actually want this... >>> >>> Nevermind, the main problem is that _any_ change in this area can break >>> something. This code is sooooooo old. >>> >>> But let me repeat, I think this change is fine anyway. >>> >>> Acked-by: Oleg Nesterov <[email protected]> >> >> Awesome, thanks! > > Hi folks, > > Can't help but notice this didn't make it into rc3. Not sure if it's > bubbling up somewhere I can't see, but we'd really like this to get > into 4.8 so we don't have to work around the regression. > > Thanks!
Eek, thanks for the ping. I got distracted by some other bugs fixes to get landed, but I've sent the seccomp pull request to James now. It should show up for -rc4. Thanks! -Kees -- Kees Cook Nexus Security
