commit b7e7fa9643d24c01d4757844bf0c820b9fe1c978
author James Sullivan <jfsulliv@google.com> Wed Feb 12 12:08:31 2025 -0800
committer CQ Bot <fuchsia-internal-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Feb 12 12:08:31 2025 -0800
tree 930e24de56decdeaa3fef6170f12b2dfacde40f7
parent deed96b764c2757f449f286b8f3a591414a121fb

[fuchsia-async] Fix timer race

It was possible for timers to become indefinitely blocked, due to a bug
in the timer heap.

Consider two threads, one of which is in Timers::poll (T1) and the other
of which is in Timers::wake_timers_impl (T2):

T2: Remove the timer from the heap.
T2: Take the most recently registered waker.  There was none registered
    (perhaps because the timer expired before it was polled), so no task
    will wake.
T1: Register a waker.
T1: Load state; see that state is not FIRED, so return Poll::Pending.
    (Task is now blocking, waiting for someone to call `wake`)
T2: Store state to FIRED.
T2: Drop the timer.

Now, there is a waker registered, but the timer is no longer in the
heap, so `Waker::wake()` will never be called, which leads the task that
T1 was polling to block indefinitely.

This can be avoided by ensuring that T2 stores the FIRED state *before*
it takes the waker, which is the first half of this fix.  (Note that
`AtomicWaker::take` is a Release barrier so we can be certain that the
store of FIRED won't be ordered after it.)

However, that conflicts with the fast-path in `Timers::unregister`,
where we elide taking the lock on `inner` if the timer state is anything
but REGISTERED.  That is no longer safe, because if `unregister` (which
is called in Drop) exits without blocking on `inner`, then the timer's
resources could be freed, and then the other thread might access them,
which is a UAF.

I tested this fix with a reproducer that is reliable locally, but I've
chosen to not land the reproducer as it is very expensive and
high-level.

Bug: 392098305
Change-Id: I88af2097a9ec30304c7925a8efbb360a02a387a7
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1203788
Commit-Queue: James Sullivan <jfsulliv@google.com>
Reviewed-by: Chris Suter <csuter@google.com>