Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: e25bd0f0ddc7f8c84f3c1074a89ab1bfaf27e3e5
https://github.com/WebKit/WebKit/commit/e25bd0f0ddc7f8c84f3c1074a89ab1bfaf27e3e5
Author: Robert Jenner <[email protected]>
Date: 2026-08-28 (Fri, 28 Aug 2026)
Changed paths:
M
Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h
M
Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.mm
Log 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: [email protected] (d586a22ef92e).
rdar://185367169
Canonical link: https://commits.webkit.org/320096@main
Commit: b0d927d67567bd5d7c7da14695ced80ad287c3e9
https://github.com/WebKit/WebKit/commit/b0d927d67567bd5d7c7da14695ced80ad287c3e9
Author: Abrar Rahman Protyasha <[email protected]>
Date: 2026-08-28 (Fri, 28 Aug 2026)
Changed paths:
M
Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.mm
Log Message:
-----------
REGRESSION(316606.145@safari-7625-branch): UI process crash under DisplayLink
callbacks for RemoteLayerTree DisplayLink clients
rdar://183949932
Reviewed by Charlie Wolfe.
The UI process crashes on the CVDisplayLink thread in when calling
displayLinkFired() on a DisplayLink::Client that was freed while still
registered. Two independent teardown paths can leave a client registered
at destruction. These are:
1. A client would not be removed if existingDisplayLink() returned null,
this was fixed in 314099@main.
2. More importantly, 316606.145@safari-7625-branch dropped the if (mED)
guard around startOrStopDisplayLink() when stopping the
displayDidRefresh callbacks. That method is re-entered during teardown
since invalidate() destroys mED and its destructor calls back through
into stopDisplayDidRefreshCallbacks(). Without the guard,
startOrStopDisplayLink() re-adds the DisplayLink client that
invalidate() just removed via removeDisplayLinkClient(). The client is
then freed, so the next display refresh invokes displayLinkFired() on
a freed client, producing a UAF.
In this patch, we fix the latter by re-introducing the if (mED) guard
dropped in the regressing commit.
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.mm:
(WebKit::RemoteLayerTreeEventDispatcher::stopDisplayDidRefreshCallbacks):
Originally-landed-as: 7a268c48ba0a. rdar://185365990
Canonical link: https://commits.webkit.org/320097@main
Compare: https://github.com/WebKit/WebKit/compare/87a413dfec88...b0d927d67567
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
