> > in check_dead_utrace -- it can happen if utrace_clear_tsk() skipped
> > clearing ->utrace pointer in otherwise normal detaching sequence. This
> > can happen, due to utrace->u.live.signal being valid pointer at that time.
> > Now this can happen when execution of resumed task starts:
> >
> > do_notify_resume
> > get_signal_to_deliver
> > tracehook_get_signal
> > utrace_get_signal
> > [utrace pointer found, utrace->lock taken]
> [> "not taken", of course.]
> > utrace_quiescent
> > [signal is valid here, put onto live struct utrace without
> > locking]
> >
> > So, ->utrace clearance skipped
> > wake_quiescent
> > check_dead_utrace
> > BUG_ON(tsk->utrace == utrace);
> Again let me conjure the theory on what's supposed to prevent this one,
> and maybe you will see the holes in the logic (or maybe I will).
>
> utrace_detach holds the utrace lock. To be in the remove_engine and
> wake_quiescent path, quiesce returned 1, meaning it was in TASK_TRACED.
> After that remove_engine called utrace_clear_tsk, and u.live.signal was
> set then. The theory is that when check_dead_utrace checks u.live.signal
> it will match what utrace_clear_tsk saw. When it sees something there,
> it doesn't take the path to rcu_utrace_free. The BUG_ON hits because
> the theory does not hold.
>
> Since we hold the lock, the only thing waking the target up before us is
> SIGKILL. When that happens, utrace_quiescent will clear u.live.signal
> asynchronously without regard to utrace->lock. If this happens after
> utrace_clear_tsk and before check_dead_utrace, then we hit the anomaly.
>
> Do you think that logic is sound? The original theory was based on the
> expectation that nothing else de-quiesces the target while we hold the
> lock. I think that's still sound with the exception of SIGKILL.
> Do you think that is valid otherwise?
>
> If this logic does explain the problem, then perhaps it is fixed by the
> patch below. The comment gives the rationale for why it should cover the
> SIGKILL scenario. What do you think?
> --- a/kernel/utrace.c
> +++ b/kernel/utrace.c
> @@ -1423,6 +1423,23 @@ restart:
> */
> BUG_ON(tsk->utrace != utrace);
> BUG_ON(utrace->u.live.signal != signal);
> + if (unlikely(killed)) {
> + /*
> + * Synchronize with any utrace_detach that
> + * might be in progress. It expected us to
> + * stay quiescent, but SIGKILL broke that.
> + * Taking this lock here serializes its
> + * work so that if it had the lock and
> + * thought we were still in TASK_TRACED, we
> + * block until it has finished looking at
> + * utrace. A utrace_detach that gets the
> + * lock after we release it here will not
> + * think we are quiescent at all, since we
> + * are in TASK_RUNNING state now.
> + */
> + spin_lock(&utrace->lock);
> + spin_unlock(&utrace->lock);
> + }
> utrace->u.live.signal = NULL;
> }
How can this fix anything? utrace->u.live.signal was still set to
valid on-stack pointer many lines above without any exclusion from
utrace_detach().
