Re: [PATCH] signal: Always attempt to allocate siginfo for SIGSTOP
From: Kees Cook
Date: Tue Feb 05 2019 - 09:49:22 EST
On Tue, Feb 5, 2019 at 2:17 PM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> 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 <jackoalan@xxxxxxxxx> 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 <tycho@xxxxxxxx>
> Reported-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Reported-by: Jack Andersen <jackoalan@xxxxxxxxx>
> Acked-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Christian Brauner <christian@xxxxxxxxxx>
> 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" <ebiederm@xxxxxxxxxxxx>
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!
> 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;