On 10/06, Jan Kratochvil wrote:
>
> On Tue, 06 Oct 2009 15:10:10 +0200, Oleg Nesterov wrote:
> > On 10/06, Jan Kratochvil wrote:
> > > On Mon, 05 Oct 2009 21:00:37 +0200, Oleg Nesterov wrote:
> > > > On 10/05, Jan Kratochvil wrote:
> > > > > Naive programs expect the first signal after PTRACE_ATTACH will be
> > > > > SIGSTOP.
> > > >
> > > > They should not, this is just wrong.
> > >
> > > That may be a right point but such programs are in use out there. Sure
> > > if it
> > > would be a real difficulty one can keep it as-is as GDB-7.0 soon to be
> > > released has it already fixed, strace works with it. Still ltrace
> > > crashes the
> > > inferior in such case.
> >
> > Confused. Do you mean we should fix the kernel to match this
> > expectation?
>
> Yes, I was thinking it would be a good idea.
OK. Let's see what Roland thinks.
Hmm. I guess with utrace it will be simple to do this change...
Instead of sending SIGSTOP, PTRACE_ATACH can mark the new ptrace_context
as "just attached" and trigger UTRACE_INTERRUPT, ptrace_report_signal()
can synthesize SIGSTOP itself. But not in the first version.
> > > > This test-case also does:
> > > >
> > > > /* detach with SIGPIPE/attach. This should kill tracee */
> > > > ptrace (PTRACE_DETACH, child, (void *) 1, (void *) SIGPIPE);
> > > >
> > > > ptrace (PTRACE_ATTACH, child, (void *) 0, (void *) 0);
> > > >
> > > > waitpid (child, &status, 0);
> > > > assert (WIFSIGNALED (status) && WTERMSIG (status) == SIGPIPE);
> > > >
> > > > It fails if the second PTRACE_ATTACH sees SIGPIPE. This is what
> > > > I can't understand.
>
> Second keyword "PTRACE_ATTACH" is on line 167.
> First keyword "SIGPIPE" is on line 199.
> Line 167 cannot see anything from line 199.
>
> Assuming you did mean "third PTRACE_ATTACH".
Yes,
> Line 222: if (WIFSIGNALED (status) && WTERMSIG (status) == SIGPIPE)
> Line 227: assert (WIFSTOPPED (status));
> Line 228: assert (WSTOPSIG (status) == SIGSTOP);
>
> Yes, I agree with the current general behavior of ptrace there is missing:
> if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGPIPE)
Hmm, not sure, but I am almost sleeping ;) I mean, the test-case
looks correct wrt current implemantation of ptrace. But see below.
> > It fails on ptrace-over-utrace.
>
> Attached, I do not have kernel.rpm with ptrace-over-utrace ready, please
> verify it and check it in (or ping me or so).
ptrace-over-utrace is under development, not finished yet.
> > Once again. Suppose that the tracer does ptrace(PTRACE_DETACH, SIGXXX).
> > Currently, if the next thacer attaches right after this detach it has no
> > way to intercept SIGXXX, it will be never reported via ptrace_signal().
>
> No matter if it gets reported to the new tracer still SIGXXX should never get
> lost.
Great.
> If it is not reported to the new tracer then it will be always
> processed by the tracee, is it right?
Yes, sure.
(But, just in case... if the tracer does ptrace(DETACH, SIGNR), this
signr only matters if the tracee was stopped after reporting syscall
or signal, otherwise SIGNR is ignored).
> > OK, this relates to "the first signal should be SIGSTOP" but this
> > is wrong anyway, and the case above is very unlikely.
>
> If "the first signal should be SIGSTOP" is not satisfied (current state)
> I think you can freely change this behavior whether SIGXXX will be reported to
> the new tracer and we should apply the attached ptrace-testsuite patch.
Good.
> If "the first signal should be SIGSTOP" gets fixed/satisfied (proposed state)
> I think it is clear SIGXXX from PTRACE_DETACH must not be lost and it must not
> be visible as the first signal after PTRACE_ATTACH.
Agreed.
> --- attach-into-signal.c 31 Jan 2009 21:11:40 -0000 1.5
> +++ attach-into-signal.c 6 Oct 2009 14:27:08 -0000
> @@ -224,6 +224,18 @@ static void reproduce (void)
> child = 0;
> return;
> }
> + /* SIGPIPE was still pending and it has not been yet delivered. */
> + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGPIPE)
> + {
> + /* Deliver it and get the queued SIGSTOP. */
> + errno = 0;
> + ptrace (PTRACE_CONT, child, (void *) 1, (void *) SIGPIPE);
> + assert_perror (errno);
> +
> + errno = 0;
> + pid = waitpid (child, &status, 0);
> + assert (pid == child);
> + }
Yes, I didn't verify this yet, but I think with this patch the
test-case should succeed with utrace-ptrace kernel.
Thanks Jan.
Oleg.
