On 03/12, Roland McGrath wrote:
>
> > Yep. And utrace_reset() can be called because ->stopped == 1.
>
> Right.
>
> > Let me explain. Again, let's suppose D attaches engine E to the target T.
> >
> > T enters utrace_report_jctl() with ->stopped == 1.
> >
> > D calls utrace_set_events(events => 0), this removes JCTL from E->flags.
> >
> > D calls, say, utrace_control(UTRACE_RESUME). Since ->stopped == 1, this
> > calls utrace_reset() and removes JCTL from T->utrace_flags.
>
> Right. In the utrace-indirect code this would have reset the utrace
> pointer too.
>
> > T takes utrace->lock, clears ->stopped, and drops the lock.
>
> In the utrace-indirect code, this part would have been harmless even in the
> race case where it happened (the more likely case being that task->utrace
> was cleared already before utrace_report_jctl looked at it). (That code
> just had the dangling utrace pointer issue I noticed yesterday, at the end
> of the function.)
>
> But, yes, this is a problem. I think this ought to cover it:
>
> @@ -1659,6 +1659,16 @@ void utrace_report_jctl(int notify, int what)
> * longer considered stopped while we run callbacks.
> */
> spin_lock(&utrace->lock);
> + /*
> + * Now that we have the lock, check in case utrace_reset() has
> + * just now cleared UTRACE_EVENT(JCTL) while it considered us
> + * safely stopped. In that case, we should not touch ->stopped
> + * and have nothing else to do.
> + */
> + if (unlikely(!(task->utrace_flags & UTRACE_EVENT(JCTL)))) {
> + spin_unlock(&utrace->lock);
> + return;
I don't think this can help, even if we clear ->stopped before return.
It is still possible to set ->stopped after that, and since we don't
have JCTL we return from get_signal_to_deliver() bypassing tracehook
calls.
>From the previous message:
>
> That suggests we must preemptively go back to TASK_RUNNING before making
> the callbacks, just in case they would do the transition.
> ...
I thought about this too. But this not easy and not nice.
Roland, I _seem_ to have the vague idea, will return tomorrow.
Oleg.
