Messages in this thread | | | Date | Tue, 25 Mar 2008 19:33:16 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH] Discard notification signals when a tracer exits |
| |
On 03/25, Oleg Nesterov wrote: > > This patch needs Roland's opinion. I can't really judge, but I > have some (perhaps wrong) doubts. > > On 03/25, Petr Tesarik wrote: > > > > When a tracer exits without detaching from the traced process, the > > tracee may be at a tracer notification stop and will thus interpret > > the value in task->exit_code (SIGTRAP | 0x80) as the signal to be > > delivered. > > > > This patch fixes the problem by clearing exit_code when detaching > > the traced process from a dying tracer. > > > > Signed-off-by: Petr Tesarik <ptesarik@suse.cz> > > > > --- > > exit.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > --- a/kernel/exit.c > > +++ b/kernel/exit.c > > @@ -642,8 +642,10 @@ reparent_thread(struct task_struct *p, s > > /* > > * If it was at a trace stop, turn it into > > * a normal stop since it's no longer being > > - * traced. > > + * traced. Cancel the notification signal, > > + * or the tracee may get a SIGTRAP. > > */ > > + p->exit_code = 0; > > ptrace_untrace(p); > > } > > } > > @@ -713,6 +715,10 @@ static void forget_original_parent(struc > > p->real_parent = reaper; > > reparent_thread(p, father, 0); > > } else { > > + /* cancel the notification signal at a trace stop */ > > + if (p->state == TASK_TRACED) > > + p->exit_code = 0; > > This reduce the likelihood that the tracee will be SIGTRAP'ed, but doesn't > solve the problem, no? > > Suppose that the tracee does send_sigtrap(current) in do_syscall_trace() > and then ptracer exits. Or ptracer wakes up the TASK_TRACED tracee without > clearing its ->exit_code and then you kill(ptracer, SIGKILL). > > If we really need this, _perhaps_ it is better to change do_syscall_trace(), > so that the tracee checks ->ptrace before sending the signal to itself. > > > But actually, I don't understand what is the problem. Ptracer has full control, > you should not kill it with SIGKILL, this may leave the child in some bad/ > inconsistent change.
Additional note. Suppose that the tracee dequeues the "good" signal, notices PT_PTRACED and calls ptrace_stop(). We set TASK_TRACED under ->siglock, without holding tasklist_lock. At this moment you kill strace, it clears ->exit_code. The tracee notices it is not traced any longer and returns to get_signal_to_deliver(). Since ->exit_code is cleared, the "right" signal is lost.
So I think this patch adds a race. In some sense (yes I am biased) this is even worse than the problem this patch tries to solve, because this race is unlikely and is hard to trigger/debug, and it could be easily unnoticed.
Oleg.
| |