On 10/29, Roland McGrath wrote:
>
> > void utrace_finish_jctl(void)
> > {
> > struct utrace *utrace = task_utrace_struct(current);
> > /*
> > * While in TASK_STOPPED, we can be considered safely stopped by
> > * utrace_do_stop(). Make sure we can do nothing until the
> > tracer
> > * drops utrace->lock
> > */
> > if (unlikely(__fatal_signal_pending()))
> > spin_unlock_wait(utrace->lock);
> > }
> >
> > and utrace_stop() should do the same.
>
> I see. The comments should be more clear that SIGKILL breaking
> TASK_TRACED is the only way this can arise. In the jctl case, that is
> in particular after utrace_do_stop() changed TASK_STOPPED into TASK_TRACED.
Agreed.
> So today ->stopped really does still mean one other thing. It means
> that it's either still in TASK_TRACED or is just waking up for SIGKILL.
>
> I think we've discussed the related stuff before. A contrary approach
> would be to ensure that in the SIGKILL-breaks-UTRACE_STOP case we never
> make any more normal reports at all before utrace_report_death.
Yes, I agree.
> I was attracted by this until I started going through it.
> It's all good until you consider report_clone. If a SIGKILL is pending
> when you get to utrace_report_clone(), the clone has already happened.
> If you skip that callback, the engine doesn't find out about the new
> child, and that matters if it's not a CLONE_THREAD.
another nasty case is report_exit(). Even if we fix the discussed problems
with explicit/implicit SIGKILL's. We shouldn't afaics skip this report if
the tracee was killed by execing sub-thread.
Both can be fixed if we add spin_unlock_wait() before REPORT(). But imho
it would be safer if we start with spin_unlock_wait() in utrace_stop(),
perhaps there is something else to consider.
Oleg.
