On 08/18, Roland McGrath wrote:
>
> > 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.
>
> Your implementation of this can probably just use the same method as for a
> real PTRACE_DETACH,signr.  Just pick up last_siginfo->si_signo and use it.

Yes. Or we can simplify the code a bit (check ->ptrace in ptrace_attach_task)
and make sure that after UTRACE_ATTACH_EXCLUSIVE ptrace_attach() can never
fail and thus we should worry about detaching at all.

Unless the tracee is killed, of course. But in this case we don't care and
just return after "if (->exit_state)" check.

> > 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. [...]
>
> I don't really understand why the SIGSTOP is your concern.
> No matter what, you send that last, after ptrace is wired up to catch it.

Because the tracee must not dequeue and handle this SIGSTOP until it sees
UTRACE_EVENT_SIGNAL_ALL in ->utrace_flags.

> > ptrace_attach_task() can check ->ptrace == 0 ! Something like
>
> Sorry, I've lost track of what this flag (that really shouldn't exist at
> all) means in your version that's distinct from whether you have the
> ptrace_utrace_ops engine attached.

Yes, ->ptrace should die. But we can't kill it right now. (I mean, I don't
see how can we kill it before we have a more or less working ptrace code).

Until we kill it, ptrace_attach pathes should

        - set PT_PTRACED for task_ptrace() callers in exit/signal/etc.c
        
        - check it to avoid the races with another tracer doing detach,
          iow simply to prevent __ptrace_link() before __ptrace_unlink().

So, ptrace_attach() can do

        utrace_attach_task(UTRACE_ATTACH_EXCLUSIVE, ptrace_utrace_ops);

        if (tracee->ptrace) {
                // We race with detach: a previous tracer already detached
                // its engine, but we can't __ptrace_link() yet.
                return -EPERM;
        }

        // Nobody can attach ptrace_utrace_ops now. This also means
        // nobody can set ->ptrace != 0 except us.

        utrace_set_events(...);

        write_lock(tasklist);
        tracee->ptrace = PT_PTRACED;
        __ptrace_link();
        write_unlock(tasklist);

Oleg.

Reply via email to