[6] DeferredWorkTimer queued duplicate raw tickets during realm teardown

MediumJSC runtimeOther

A teardown path that added work to the queue it was supposed to drain

e7c6375

Medium. The removed line was pure redundancy — the drain loop's trailing cancelled-ticket sweep already did the job — but it left a second queue entry pointing at an address whose only owner had just been released. Escalation needs the freed slot refilled by another ticket before the stale entry is drained.

A work queue that names its subjects by raw address is safe only for as long as something else keeps those objects alive at least as long as the queue entries. DeferredWorkTimer is the JavaScript engine's run-loop timer for deferred work — WebAssembly compile promises, Atomics.waitAsync resolutions, FinalizationRegistry cleanup — and it splits that job across two containers with different ownership strength: one that owns tickets by strong reference, and one that queues them by bare pointer alongside the callable to run. The implicit invariant is that every raw ticket sitting in the queue is backed by a live ticket still held in the owning set, and that each ticket appears there at most once per outstanding owner.

The angle: a page that grooms the ticket allocator around a realm teardown could get the work timer to consume and destroy a live, unrelated ticket, leaving that ticket's own queued entry dangling.

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 at the end that purges cancelled tickets from m_pendingTickets without needing an m_tasks entry, and setTimeUntilFire(0_s) already ensures doWork() fires to run that cleanup. Originally landed as 305413.677@safari-7624-branch.

Source/JavaScriptCore/runtime/DeferredWorkTimer.cpp

Source/JavaScriptCore/runtime/DeferredWorkTimer.cpp (context, unchanged — doWork())

JSTests/wasm/stress/deferred-work-timer-cancel-duplicate-ticket.js

Patch Details

A single-line deletion in DeferredWorkTimer::cancelPendingWorkSafe(JSGlobalObject*). The function iterates *globalObject->m_weakTickets, cancelling every still-live ticket via cancelPendingWork(ticket.ptr()), and then — pre-fix — unconditionally appended std::make_tuple(ticket.ptr(), [](DeferredWorkTimer::Ticket) { }), a raw Ticket paired with a noop Task, to the m_tasks deque for every weak ticket of the dying global, including tickets that already had an entry from a prior scheduleWorkSoon(). The patch removes that append and keeps the trailing if (!isScheduled() && !m_currentlyRunningTask) setTimeUntilFire(0_s);, so doWork() still fires. The regression test drives a child global with 300 FinalizationRegistry instances plus Atomics.waitAsync tickets, drops the global, then floods the heap with setTimeout work and repeated gc() calls.

Queue entries naming an object by raw address while its sole owning reference lives in a separate container, so a second entry outlives the owner and re-resolves against a new occupant.

Background

DeferredWorkTimer. A JSRunLoopTimer owned by the VM that runs deferred work on the run loop. Clients call addPendingWork() to get a ticket, then scheduleWorkSoon(ticket, task) when the work is ready to run; doWork() drains the queue on a later run-loop turn.

TicketData and Ticket. TicketData is a ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr object holding the work's GC dependencies (FixedVector<JSCell*> m_dependencies), a script-execution owner, and a cancellation flag; it is TZone-allocated. Ticket is a type alias for a bare TicketData* — an unmanaged raw pointer.

Two containers, two ownership strengths. UncheckedKeyHashSet<Ref<TicketData>> m_pendingTickets holds strong references and is the ticket's owner; Deque<std::tuple<Ticket, Task>> m_tasks, guarded by m_taskLock, holds raw pointers plus the callable to run.

Weak ticket registry. Each TicketData registers itself with its realm in its constructor via target()->realm()->addWeakTicket(this), so a JSGlobalObject can enumerate the tickets belonging to it. cancelPendingWorkSafe(JSGlobalObject*) is called during JSGlobalObject teardown; it marks the realm's outstanding tickets cancelled and pokes the timer with setTimeUntilFire(0_s) so doWork() runs the cleanup sweep.

The drain loop. For each queued entry, doWork() looks the raw ticket up in m_pendingTickets, skips entries not found, removes cancelled ones, drops the lock while running the task, and finishes with m_pendingTickets.removeIf(isCancelled) to purge tickets cancelled without ever being scheduled.

Ticket producers reachable from script. Atomics.waitAsync (resolved by Atomics.notify), WebAssembly async compile/instantiate promises, and FinalizationRegistry cleanup scheduling.

TZone allocation. WebKit's type-segregated heap, so a freed object's slot is reused by another allocation of the same type — here, another TicketData.

Analysis

This is a stale-raw-pointer bug that presents as an ABA-style identity confusion: a queue entry keyed by an address whose sole owner is dropped while another entry to the same address is still queued.

  m_tasks (raw TicketData*)          m_pendingTickets (Ref<TicketData>)
  ─────────────────────────          ─────────────────────────────────
  [T, realTask]  (scheduleWorkSoon)   { T }
  [T, noop]      (teardown append)    <-- the removed line
  doWork() entry 1: T cancelled -> m_pendingTickets.remove(T) -> free(T)
  doWork() entry 2: find(T) keyed on a freed address
      miss -> skipped harmlessly
      hit  -> a NEW ticket T' occupies the slot: take/consume/destroy T'
              and T''s own real queue entry is now the dangling one

For a ticket that already had a real entry queued by scheduleWorkSoon(), the teardown path added a second entry for the same address with no strong reference attached and no check for an existing entry. doWork() processes the first entry, sees ticket->isCancelled() — the loop above set it — and executes m_pendingTickets.remove(pendingTicket), dropping the last Ref<TicketData> and destroying the object. The duplicate entry remains, holding the freed address. Hashing does not dereference, so the common case is a lookup miss and a harmless skip.

The hazardous case is address reuse. If a freshly created TicketData lands in the freed TZone slot — and the test's setTimeout flood plus repeated gc() is precisely a slot-churn generator — the lookup matches the unrelated live ticket, ASSERT(ticket == pendingTicket->ptr()) passes because the addresses are equal, and doWork() dereferences and consumes a ticket that was never the one queued: m_pendingTickets.take(pendingTicket) removes it, the noop task runs, ticketData = nullptr destroys it, and the victim ticket's own real queued entry is left dangling. A secondary, non-racy consequence of the same append is unbounded m_tasks growth on realm teardown — one entry per weak ticket of the dying global, 300+ in the test — which is what makes the address-reuse window practically wide.

The body of cancelPendingWork(Ticket) is truncated out of the supplied source, so the reading that it only sets the cancelled flag and leaves the ticket in m_pendingTickets rests on doWork()'s trailing comment about clients that "cancel a pending ticket and never call scheduleWorkSoon()" together with the removeIf(isCancelled) sweep.

This vulnerability weakens memory safety inside the JavaScript engine's asynchronous work scheduler. The assumption at stake is that the work queue never holds a raw Ticket whose owning Ref<TicketData> has already been released — an invariant the pre-fix teardown path broke on every realm destruction that had already-scheduled tickets. An attacker who groomed the TZone allocator so a freed slot is reoccupied before the stale entry is drained could cause doWork() to consume and destroy an unrelated live ticket, corrupting the lifetime of engine-internal objects and yielding a use-after-free primitive in the WebContent process. The fix restores the invariant by never enqueuing a ticket the trailing sweep already handles.

The structural weakness is not the removed line but the split ownership model that made it dangerous: m_pendingTickets holds Ref<TicketData> while m_tasks holds bare TicketData*, so any code path appending to m_tasks is implicitly asserting a lifetime it does not own. doWork() already carries two comments acknowledging the fragility, and the find(ticket) miss-check is precisely a defence against stale entries — a defence that silently fails under address reuse because the hash key is the pointer value. The durable fix for this class would be to store a Ref or ThreadSafeWeakPtr<TicketData> in the queue tuple, since the type already derives from ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr, rather than relying on every producer of queue entries to reason about ownership correctly. It is also notable that the removed code was pure redundancy — a defensive-looking addition duplicating existing cleanup while introducing a new lifetime hazard.

Audit directions

• A work queue referencing its subjects by raw address while the only owning reference lives in a different container. Narrow: audit every writer of DeferredWorkTimer::m_tasks — scheduleWorkSoon, didResumeScriptExecutionOwner, the suspendedTasks re-prepend at the end of doWork, and any remaining teardown-path appends — and confirm each enqueued Ticket is guaranteed to be in m_pendingTickets for as long as the entry survives. In review, a std::make_tuple(x.ptr(), ...) or raw-pointer append where the strong Ref is dropped at the end of the enclosing scope is the tell. Wider: the same class appears anywhere JSC/WebCore pairs a strong-ref registry with a raw-pointer schedule — MicrotaskQueue entries, JSRunLoopTimer subclasses that queue work by pointer, and WeakGCMap/WeakGCSet-backed schedulers where lookup is by cell address; the tell is a container whose element type is T* or std::tuple<T*, ...> while a sibling member is HashSet<Ref<T>> of the same type. Widest: a deferred-work record must own, or provably outlive-check, the object it names; carry it into any codebase with a task queue keyed by pointer or index and ownership held elsewhere — Chromium's base::TaskRunner closures over raw_ptr/WeakPtr, Rust's slotmap/generational-index arenas, and epoll/kqueue registrations carrying a raw void* udata. Match tell there: a lookup that recovers an object from a bare address or index and then trusts identity without a generation or epoch check.
• Teardown paths that add work instead of only removing it. Narrow: grep Source/JavaScriptCore/runtime for functions called from JSGlobalObject/VM teardown that append to a queue or schedule a timer — cancelPendingWorkSafe, cancelPendingWork(VM&), stopRunningTasks, and the setTimeUntilFire(0_s) pokes — and check whether the enqueued payload can be drained after the realm's objects are gone; the tell is an append/prepend/schedule call lexically inside a cancel/stop/destroy function. Wider: the same shape recurs in WebCore teardown — ScriptExecutionContext stop/suspend paths, ActiveDOMObject::stop() implementations that post a task, and document detach handlers that dispatch rather than drain; the tell is a stop/detach handler whose body contains a post/dispatch/enqueue instead of a purge. Widest: shutdown must be a monotone drain — teardown may cancel and remove, never enqueue; applies to Node.js beforeExit handlers that schedule new work, Go context cancellation paths that spawn goroutines, and Rust Drop impls that push onto a shared queue. Match tell: any cancellation routine whose net effect on a queue's length is positive.
• Hash lookups keyed on a raw pointer whose address can be recycled by a type-segregated allocator. This turns a "not found, skip safely" guard into a silent false match. Narrow: review doWork()'s m_pendingTickets.find(ticket) / ASSERT(ticket == pendingTicket->ptr()) guard and any other JSC lookup that uses a T* as a key into a HashSet<Ref<T>>, asking whether a miss is the only failure mode assumed; the tell is find(rawPtr) on a container of Ref<T>/RefPtr<T> followed by code that treats a hit as proof of identity. Wider: extend to WebKit's TZone/IsoHeap-allocated types generally (WTF_MAKE_TZONE_ALLOCATED classes used as map keys) and to ObjectIdentifier-style tables where a stale identifier could be reissued; the tell is a type that is both TZone-allocated and used as a lookup key by address. Widest: pointer identity is not object identity once the allocator can recycle the address — identity checks need a generation counter, a UUID, or a strong reference; transfers to Windows HANDLE recycling, POSIX fd reuse after close, and generational indices in ECS engines. Match tell: an identity comparison whose only evidence is equality of a reusable address or handle value.