← WebKit — 2026-W35
e25bd0f0ddc7f8c84f3c1074a89ab1bfaf27e3e5 RemoteLayerTreeEventDispatcher: add lock to guard access to m_momentumEventDispatcher
severity medium
class Race
confidence 0.7
signal score 11.34
Source/WebKit/UIProcess
patch-now
Primitive: Unsynchronized access/teardown race on m_momentumEventDispatcher between display-link thread and main thread
Triage note: Adds Lock m_momentumEventDispatcherLock guarding m_momentumEventDispatcher plus null-checks at every use (handleWheelEvent, displayDidRefresh, pageScreenDidChange, flushLog, setScrollingAccelerationCurve) in RemoteLayerTreeEventDispatcher, where it is nulled on one path while dereferenced unconditionally on others. Fixes a data race / use-after-null on the dispatcher reachable via scroll event handling.
Triage note: Adds Lock m_momentumEventDispatcherLock guarding m_momentumEventDispatcher plus null-checks at every use (handleWheelEvent, displayDidRefresh, pageScreenDidChange, flushLog, setScrollingAccelerationCurve) in RemoteLayerTreeEventDispatcher, where it is nulled on one path while dereferenced unconditionally on others. Fixes a data race / use-after-null on the dispatcher reachable via scroll event handling.
Structural signals: smartptr_member_added (+3.0), null_guard (+2.59), uaf_lifetime (+4.75), msg_soft (+1.0)
Commit message
RemoteLayerTreeEventDispatcher: add lock to guard access to m_momentumEventDispatcher
https://bugs.webkit.org/show_bug.cgi?id=318487
rdar://174662710
Reviewed by Simon Fraser.
m_momentumEventDispatcher can be accessed from two threads, which could lead to
a race condition:
* On the scrolling thread, RemoteLayerTreeEventDispatcher::didRefreshDisplay
gets called. It grabs the pointer of m_momentumEventDispatcher to call into it
* Just before m_momentumEventDispatcher->displayDidRefresh is called,
the scrolling thread gets pre-empted for the main thread. It calls
RemoteLayerTreeEventDispatcher::invalidate, which sets m_momentumEventDispatcher
to nullptr, freeing the object
* Then the scrolling thread resumes execution at
RemoteLayerTreeEventDispatcher::didRefreshDisplay, which calls
calls m_momentumEventDispatcher->displayDidRefresh on the freed object
Note the race window is *very* small - didRefreshDisplay needs to be preempted
at the exact instruction that calls m_momentumEventDispatcher->displayDidRefresh.
263321@main tweaked the order of operations to avoid needing a lock and null
check around m_momentumEventDispatcher:
> Rather than adding a lock to use around accesses to m_momentumEventDispatcher
> and adding null checks, fix the teardown order to ensure that m_momentumEventDispatcher
> is always non-null in the scrolling thread code.
But it's almost too easy to accidentally change the order of operations, so this
patch properly fixes it by guarding m_momentumEventDispatcher with a lock. Any
operations involving it needs to acquire the lock first.
No tests - tiny race window makes it almost impossible to write a test that
reliably triggers the race condition.
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.mm:
(WebKit::RemoteLayerTreeEventDispatcher::invalidate):
(WebKit::RemoteLayerTreeEventDispatcher::cacheWheelEventScrollingAccelerationCurve):
(WebKit::RemoteLayerTreeEventDispatcher::scrollingThreadHandleWheelEvent):
(WebKit::RemoteLayerTreeEventDispatcher::didRefreshDisplay):
(WebKit::RemoteLayerTreeEventDispatcher::windowScreenDidChange):
(WebKit::RemoteLayerTreeEventDispatcher::stopDisplayDidRefreshCallbacks):
(WebKit::RemoteLayerTreeEventDispatcher::flushMomentumEventLoggingSoon):
Originally-landed-as: 305413.1103@safari-7624.5-branch (d586a22ef92e). rdar://185367169
Canonical link: https://commits.webkit.org/320096@main
Before / after
Loading diff…
On This Page