Kees Cook <keesc...@chromium.org> writes: > On Thu, Dec 6, 2018 at 1:11 PM Eric W. Biederman <ebied...@xmission.com> > wrote: >> >> Tycho Andersen <ty...@tycho.ws> writes: >> >> > On Thu, Dec 06, 2018 at 10:48:39AM -0800, Linus Torvalds wrote: >> >> On Thu, Dec 6, 2018 at 6:40 AM Eric W. Biederman <ebied...@xmission.com> >> >> wrote: >> >> > >> >> > We have in the past had ptrace users that weren't just about debugging >> >> > so I don't know that it is fair to just dismiss it as debugging >> >> > infrastructure. >> >> >> >> Absolutely. >> >> >> >> Some uses are more than just debug. People occasionally use ptrace >> >> because it's the only way to do what they want, so you'll find people >> >> who do it for sandboxing, for example. It's not necessarily designed >> >> for that, or particularly fast or well-suited for it, but I've >> >> definitely seen it used that way. >> >> >> >> So I don't think the behavioral test breakage like this is necessarily >> >> a huge deal, and until some "real use" actually shows that it cares it >> >> might be something we dismiss as "just test", but it very much has the >> >> potential to hit real uses. >> >> >> >> The fact that a behavioral test broke is definitely interesting. >> >> >> >> And maybe some of the siginfo allocations could depend on whether the >> >> signal is actually ever caught or not. >> >> >> >> For example, a terminal signal (or one that is ignored) might not need >> >> siginfo. But if the process is ptraced, maybe that terminal signal >> >> isn't actually terminal? So we might have situations where we want to >> >> simply check "is the signal target being ptraced".. >> > >> > Yes, something like this, I suppose? It works for me. >> >> The challenge is that we could be delivering this to a zombie signal >> group leader. At which point we won't deliver it to the target task. >> >> Sigh it is probably time that I dig in and figure out how to avoid that >> case which we need to fix anyway because we can get the permission >> checks wrong for multi-threaded processes that call setuid and friends. >> >> Once that is sorted your small change will at least be safe. >> >> Eric >> >> > From 3bcaadd56ebb532ab4d481556fcc0826d65efc43 Mon Sep 17 00:00:00 2001 >> > From: Tycho Andersen <ty...@tycho.ws> >> > Date: Thu, 6 Dec 2018 12:15:22 -0700 >> > Subject: [PATCH] signal: allocate siginfo when a traced task gets SIGSTOP >> > >> > Tracers can view SIGSTOP: >> > >> > https://lore.kernel.org/lkml/87zhtthkuy....@xmission.com/T/#u >> > >> > so let's allocate a siginfo for SIGSTOP when a task is traced. >> > >> > Signed-off-by: Tycho Andersen <ty...@tycho.ws> >> > --- >> > kernel/signal.c | 9 ++++++--- >> > 1 file changed, 6 insertions(+), 3 deletions(-) >> > >> > diff --git a/kernel/signal.c b/kernel/signal.c >> > index 9a32bc2088c9..ab4ba00119f4 100644 >> > --- a/kernel/signal.c >> > +++ b/kernel/signal.c >> > @@ -1056,11 +1056,14 @@ static int __send_signal(int sig, struct >> > kernel_siginfo *info, struct task_struc >> > goto ret; >> > >> > result = TRACE_SIGNAL_DELIVERED; >> > + >> > /* >> > - * Skip useless siginfo allocation for SIGKILL SIGSTOP, >> > - * and kernel threads. >> > + * Skip useless siginfo allocation for SIGKILL and kernel threads. >> > + * SIGSTOP is visible to tracers, so only skip allocation when the >> > task >> > + * is not traced. >> > */ >> > - if (sig_kernel_only(sig) || (t->flags & PF_KTHREAD)) >> > + if ((sig == SIGKILL) || (!task_is_traced(t) && sig == SIGSTOP) || >> > + (t->flags & PF_KTHREAD)) >> > goto out_set; >> > >> > /* > > What should we do for v4.20? I need to have the selftests actually > passing. :)
For v4.20 we need to do one of two things. 1) Present a plausible case that someone will could care about, we document it in the commit we can perform my earlier partial revert. 2) Remove the sanity check seccomp_bpf.c I really just want to ensure we have clear reasoning here. Eric
