[13] DeferredWorkTimer leaves stale ticket pointer in m_tasks after cancellation

Severity: Medium | Component: JavaScriptCore DeferredWorkTimer | e7c6375

Rated Medium because the diff fixes a stale-pointer dequeue from m_tasks where the first pop frees the TicketData and the duplicate entry dereferences the freed (or TZone-reused) pointer; reaching the window requires cross-realm teardown with sufficient in-flight tickets, and the strength of the primitive is contingent on TZone reuse landing in the freed slot.

cancelPendingWorkSafe() was unconditionally appending a (ticket, noop) entry to m_tasks for every weak ticket of a dying global. This is unnecessary because doWork() already has a removeIf(isCancelled) pass that purges cancelled tickets from m_pendingTickets, and setTimeUntilFire(0_s) already ensures doWork() fires to run that cleanup.

Source/JavaScriptCore/runtime/DeferredWorkTimer.cpp

Patch Details

A single line is removed: the unconditional append of a (ticket, noop) tuple to m_tasks for every weak ticket belonging to a dying JSGlobalObject. The remaining logic still iterates the global's m_weakTickets, calls cancelPendingWork on each non-cancelled ticket, and schedules doWork() via setTimeUntilFire(0_s). Cleanup of cancelled tickets is left to the trailing m_pendingTickets.removeIf(isCancelled) pass already at the end of doWork(). A regression test creates many FinalizationRegistry instances in a child global, drops the global, GCs, and then schedules large bursts of timer-driven deferred work.

Stale raw-pointer reuse in a work queue: an owner-side cleanup pushed duplicate raw-pointer entries that outlive the owning Ref drop done by the consumer.

Background

DeferredWorkTimer queues two structures: m_pendingTickets (a HashSet<Ref<TicketData>> holding the keep-alive reference for each ticket) and m_tasks (a Deque<tuple<Ticket, Task>> where Ticket = TicketData* is a raw pointer). Tickets are added when JS asks for deferred work (Atomics.waitAsync, FinalizationRegistry cleanup, WebAssembly promise integration); doWork() pops m_tasks FIFO under the VM API lock. TicketData::cancel() flips m_isCancelled but does not remove the ticket from either container; cleanup happens lazily inside doWork(). cancelPendingWorkSafe(JSGlobalObject*) runs from JSGlobalObject::~JSGlobalObject to clean up any tickets whose realm is dying. TicketData is allocated under WTF_MAKE_TZONE_ALLOCATED_IMPL, which means freed instances are returned to a type-segregated freelist and the next TicketData::create may reuse the exact same address. HashSet<Ref<T>>::find(rawPtr) uses pointer-value hashing/equality and does not dereference the input pointer to decide membership.

Analysis

cancelPendingWorkSafe appended a (ticket, noop) entry to m_tasks for every weak ticket of the dying global, in addition to whatever entries were already queued for those tickets. m_tasks stores Ticket as a raw TicketData*; the keep-alive Ref lives in m_pendingTickets. The mainline doWork() loop, when it sees a cancelled ticket on the first matching m_tasks entry, calls m_pendingTickets.remove(pendingTicket), which drops the last Ref and destroys the TicketData. Because the same ticket pointer was queued a second time by cancelPendingWorkSafe, the next loop iteration pops (ticket, noop) whose ticket is now a dangling TicketData*.

Audit directions