On 08/17, Roland McGrath wrote:
>
> > Note the "XXX" comment in ptrace_abort_attach(). I am not sure a simple
> > UTRACE_DETACH is always correct. We already did utrace_set_events(), we
> > must not, say, lose a signal if it comes in between.
>
> I don't follow what scenario you are concerned with here.
Note that I don't really understand why I am worried ;)
This "XXX" is just a temporary marker, to remind that perhaps we should
be careful with the future changes.
But please see below.
> UTRACE_DETACH will never cause you to "lose" anything. Either you'll get
> some callback you've enabled before the detach completes (before your
> utrace_control call, or after or simultaneous with a -EINPROGRESS return
> from it), or you won't. If you don't, the signal does its normal thing.
> If you do, you do. The callback just needs to:
> return utrace_signal_action(action) | UTRACE_DETACH;
> or
> return utrace_signal_action(action) | UTRACE_RESUME;
> to have no effect on the signal.
Suppose we have a ptraced task C, it is ->stopped.
C->parent calls ptrace_detach(0), utrace_control(UTRACE_DETACH), but
doesn't clear ->ptrace yet. UTRACE_DETACH wakes up the tracee.
(Or, C can be running, ptracer exits and detaches).
A new tracer P calls ptrace_attach(C), installs the new engine E, and
calls utrace_set_events(... | UTRACE_EVENT_SIGNAL_ALL).
C recieves a signal, notices task_utrace_flags(), calls utrace_get_signal(),
dequeues the signal, calls E->ops->ptrace_report_signal() which returns
UTRACE_STOP.
C does finish_resume_report()->utrace_stop().
P continues, ptrace_attach() takes tasklist and sees PT_PTRACED. It calls
utrace_control(E, UTRACE_DETACH).
The signal is lost.
In short. Perhaps I missed something, but I think it would be "safer" if
ptrace_attach() did utrace_set_events() after setting PT_PTRACED, but this
is not possible because attach sends SIGSTOP. That is why initially I was
going to implement attach this way:
utrace_attach_task(UTRACE_ATTACH_CREATE);
write_lock(tasklist);
if (child->ptrace)
abort();
child->ptrace = PT_PTRACED;
write_unlock(tasklist);
utrace_set_events(...);
send_sig_info(SIGSTOP);
But then decided to revert this nightmire and keep the code simple.
(Also. Unless I missed something, this means that even ptrace_detach(0)
can't always use a plain UTRACE_DETACH. But lets discuss this later).
Oleg.
