On 10/07, Roland McGrath wrote:
>
> > Currently, if a tracer does ptrace(DETACH, tracee, SIGXXX)
> > and then another/same tracer does ptrace(ATTACH, tracee)
> > then SIGXXX will not be reported to the new tracer.
>
> Correct.
Well almost...
What if the tracee reports a signal and stops, the tracer detaches
but does not wake it up because of SIGNAL_STOP_STOPPED ? In this
case this signal will be visible after the next attach.
OK, currently ptrace_detach() unconditionally wakes up the tracee.
But we seem to agree this wakeup should be killed, it completely
breaks the group-stop logic and it has other problems.
> > Why? Should utrace-ptrace be 100% compatible here?
>
> The rationale I see is that the debugger observed the signal, decided it
> wanted the tracee to experience the normal effects of the signal and be
> done with it after that. If it's a termination signal, the debugger really
> expects that the tracee can do nothing but die. (Given the tortured
> historic logic of the ptrace code paths, this applies only to signal stops
> and only when it either passes back the same signal number or at least
> another one that is not blocked.) Now replace "debugger" with "monitor" or
> (dismal) "security-checking syscall tracing product", or just shudder while
> trying not to imagine what all inane things ptrace might already be in use
> for. It could be important to someone out there e.g. that some other
> process of the tracee's UID cannot race in there and rescue it from dying.
If "monitor" really wants to kill the tracee it should use SIGKILL ;)
Any other fatal signal can be ignored by the tracee's sub-thread after
detach but before the tracee returns from ptrace_signal().
This leads to another isssue I forgot to mention when I sent
[PATCH 71] RFC, fix ptrace(PTRACE_DETACH, signr) logic
https://www.redhat.com/archives/utrace-devel/2009-October/msg00043.html
Since detach_signal() actually sends the signal, ptrace(DETACH, SIGKILL)
really works: it wakes up SIGNAL_STOP_STOPPED tracee.
> > I do not think there is a "real life" application that does
> > ATTACH + DETACH and relies on fact it must not see this sig.
>
> We can hope, but we cannot assume.
Oh, OK.
Lets discuss how we can implement detach to be compatible...
We already discussed detached_ops. So, we either change engine->ops
(my choice) or add the new engine with ->ops = detached_ops and detach
the ptrace engine. We need detached_report_signal() callback which
fixups siginfo if signr != info->si_signo and returns
UTRACE_DETACH | UTRACE_SIGNAL_DELIVER.
How can detached_report_signal() set info->si_pid correctly? the
tracer has gone away. This probably means this info should be prepared
during detach, the tracer should fill context->siginfo.
But the real problem, I can't see how can we teach ptrace_report_signal()
to not play with parting signals. Do you have any idea? perhaps we can
introduce UTRACE_SIGNAL_DELIVER_I_AM_DETACHED_ENGINE, but this is ugly.
Oh, I'd like to avoid detached_ops. Perhaps the new attach can re-use
the old engine/context?
ptrace(DETACH, SIGNR) does:
context->detached = true; // checked in ptrace_report_signal()
return utrace_control(INTERRUPT);
Now, how ptrace(ATTACH) can clear ->detached and re-use this engine without
utrace->lock ? IOW, will you agree to abuse utrace->lock for this?
OR. Perhaps we can introduce UTRACE_ATTACH_MATCH_FILTER_DATA ?
static bool engine_matches(struct utrace_engine *engine, int flags,
const struct utrace_engine_ops *ops, void
*data)
{
if ((flags & UTRACE_ATTACH_MATCH_OPS) && engine->ops != ops)
return false;
if ((flags & UTRACE_ATTACH_MATCH_FILTER_DATA)) {
bool (*filter)(void*) = data;
if (!filter(engine->data))
return false;
}
if ((flags & UTRACE_ATTACH_MATCH_DATA) && engine->data != data)
return false;
return engine->ops && engine->ops != &utrace_detached_ops;
}
Now, PTRACE_ATTACH does:
bool ptrace_filter(void *engine)
{
struct ptrace_context *context = ptrace_context(engine);
if (!context->detached)
return false;
context->detached = false; // re-use
return true;
}
ptrace_attach_task(...)
{
...
engine = utrace_attach_task(MATCH_OPS | MATCH_FILTER_DATA,
ptrace_utrace_ops,
ptrace_filter);
if (!IS_ERR(engine)) {
// We re-used the old engine, but perhaps it is
// going to return UTRACE_DETACH ?
utrace_barrier(engine);
if (engine->ops == utrace_detached_ops)
engine = -EANY;
}
if (IS_ERR(engine))
engine = utrace_attach_task(CREATE | EXCLUSIVE,
ptrace_utrace_ops,
context);
...
}
What do you think?
In any case, I'd like to think more. Somehow I am so sleepy today,
perhaps I can invent something else tomorrow... But actually I hope
you can find a better solution ;)
Oleg.
