Re: siginfo pid not populated from ptrace?
From: Eric W. Biederman
Date: Thu Dec 06 2018 - 16:11:36 EST
Tycho Andersen <tycho@xxxxxxxx> 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 <ebiederm@xxxxxxxxxxxx> 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 <tycho@xxxxxxxx>
> 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.fsf@xxxxxxxxxxxx/T/#u
>
> so let's allocate a siginfo for SIGSTOP when a task is traced.
>
> Signed-off-by: Tycho Andersen <tycho@xxxxxxxx>
> ---
> 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;
>
> /*