On Tue, Feb 5, 2019 at 2:17 PM Eric W. Biederman <ebied...@xmission.com> wrote: > > > Since 2.5.34 the code has had the potential to not allocate siginfo > for SIGSTOP signals. Except for ptrace this is perfectly fine as only > ptrace can use PTRACE_PEEK_SIGINFO and see what the contents of > the delivered siginfo are. > > Users of PTRACE_PEEK_SIGINFO that care about the contents siginfo > for SIGSTOP are rare, but they do exist. A seccomp self test > has cared and lldb cares. > > Jack Andersen <jackoa...@gmail.com> writes: > > > The patch titled > > `signal: Never allocate siginfo for SIGKILL or SIGSTOP` > > created a regression for users of PTRACE_GETSIGINFO needing to > > discern signals that were raised via the tgkill syscall. > > > > A notable user of this tgkill+ptrace combination is lldb while > > debugging a multithreaded program. Without the ability to detect a > > SIGSTOP originating from tgkill, lldb does not have a way to > > synchronize on a per-thread basis and falls back to SIGSTOP-ing the > > entire process. > > Everyone affected by this please note. The kernel can still fail to > allocate a siginfo structure. The allocation is with GFP_KERNEL and > is best effort only. If memory is tight when the signal allocation > comes in this will fail to allocate a siginfo. > > So I strongly recommend looking at more robust solutions for > synchronizing with a single thread such as PTRACE_INTERRUPT. Or if > that does not work persuading your friendly local kernel developer to > build the interface you need. > > Reported-by: Tycho Andersen <ty...@tycho.ws> > Reported-by: Kees Cook <keesc...@chromium.org> > Reported-by: Jack Andersen <jackoa...@gmail.com> > Acked-by: Linus Torvalds <torva...@linux-foundation.org> > Reviewed-by: Christian Brauner <christ...@brauner.io> > Fixes: f149b3155744 ("signal: Never allocate siginfo for SIGKILL or SIGSTOP") > Fixes: 6dfc88977e42 ("[PATCH] shared thread signals") > History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git > Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com>
Cc: stable too? In your -next tree (and the pull), can you also revert 2bd61abead58 ("selftests/seccomp: Remove SIGSTOP si_pid check") too? Thanks for getting this in! -Kees > --- > > Unless someone objects I will drop this in my tree for linux-next and > send Linus a pull request in a couple of days. > > kernel/signal.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/signal.c b/kernel/signal.c > index e1d7ad8e6ab1..45298b3a8ffc 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1057,10 +1057,10 @@ static int __send_signal(int sig, struct > kernel_siginfo *info, struct task_struc > > result = TRACE_SIGNAL_DELIVERED; > /* > - * Skip useless siginfo allocation for SIGKILL SIGSTOP, > + * Skip useless siginfo allocation for SIGKILL, > * and kernel threads. > */ > - if (sig_kernel_only(sig) || (t->flags & PF_KTHREAD)) > + if ((sig == SIGKILL) || (t->flags & PF_KTHREAD)) > goto out_set; > > /* > -- > 2.17.1 > -- Kees Cook