> Why utrace_report_jctl() and utrace_get_signal() have to clear ->stopped?
> Because unlike utrace_stop(), do_signal_stop() does not do
> finish_utrace_stop().

Correct.

> This means that utrace_report_jctl() and utrace_get_signal() might be
> called with ->stopped == true, ->stopped can be set by
> UTRACE_STOP/UTRACE_DETACH while we were stopped _before_.

By utrace_do_stop(), correct.

> (btw, I think the comment in utrace_report_jctl() above "->stopped = 0"
>  is confusing, it looks as if we are trying to prevent the race with
>  utrace_do_stop() which was called after the caller sets TASK_STOPPED).

I am a bit confused now too.  Please rewrite any comments to be more clear
as you come across them.

> Now return to the problem. It is even simpler (and worse) than I thought.
> Let's suppose the tracee is TASK_STOPPED, and the tracer does
> ptrace_check_attach(). utrace_control(UTRACE_STOP) sets ->stopped = true.
> But, the tracee is still TASK_STOPPED, not TASK_TRACED, SIGCONT can wake
> it up ?

Correct.  It will always then go through utrace_get_signal(), which is
supposed to be responsible for handling that case.

> This means ptrace_check_attach() can succeed but the tracee can even return
> to the user-space after that, no? There is no guarantee utrace_get_signal()
> will call finish_resume_report(). For example, if there are no pending
> signals, we just clear ->stopped and return after dequeue_signal().

Yes, I think you're right.  I think the "if (unlikely(utrace->stopped)) {"
case in utrace_get_signal() needs to do some variant of utrace_reset.  If
any engine_wants_stop() then it should not dequeue a signal even if one is
ready, and probably it should also not do any spontaneous report.  

OTOH perhaps it should do a spontaneous report in that case, so that if you
used utrace_control(,,UTRACE_STOP) while in TASK_STOPPED, you get a
UTRACE_SIGNAL_REPORT callback at SIGCONT time.  If that's the plan, then
all we need is utrace_do_stop()'s task_is_stopped() case to set ->report
too (and set_notify_resume for the invariant, though it's superfluous really).


Thanks,
Roland

Reply via email to