Branch: refs/heads/webkitglib/2.44
Home: https://github.com/WebKit/WebKit
Commit: b01b2f27950634fc5907200a5bdb6112e062485c
https://github.com/WebKit/WebKit/commit/b01b2f27950634fc5907200a5bdb6112e062485c
Author: Chris Dumez <[email protected]>
Date: 2024-07-31 (Wed, 31 Jul 2024)
Changed paths:
M Source/WTF/wtf/Deque.h
M Source/WebKit/Platform/IPC/Connection.cpp
Log Message:
-----------
Cherry-pick 272448.1041@safari-7618-branch (047893baba32).
https://bugs.webkit.org/show_bug.cgi?id=274597
Sync IPC messages may get processed out of order with async messages sent with
the DispatchMessageEvenWhenWaitingForSyncReply option
https://bugs.webkit.org/show_bug.cgi?id=274597
rdar://127810844
Reviewed by Geoffrey Garen and Ryosuke Niwa.
When doing a sendSync() call, the caller waits until the remote process
responds to the sync IPC.
When receiving this sync IPC, the remote process may itself decide to send sync
IPC back (or async
IPC with the DispatchMessageEvenWhenWaitingForSyncReply option) to the other
process, *before*
responding to the sync IPC. In such cases, one would expect those intermediate
IPC to be processed
*before* the reply to the sync message, to maintain ordering. Unfortunately,
our IPC logic was racy
and it was possible that the order wasn't maintained to such cases. This could
lead to logic bugs,
which sometimes translate into security bugs.
In particular, this impacted the following scenario, as proven in the radar:
1. The WebProcess sends a `WebPageProxy::ExecuteUndoRedo` sync IPC to the
UIProcess
2. The UIProcess receives the `WebPageProxy::ExecuteUndoRedo` IPC
3. The UIProcess sends an `WebPage::UnapplyEditCommand` /
`WebPage::ReapplyEditCommand` async
IPC to the WebProcess with the DispatchMessageEvenWhenWaitingForSyncReply
option.
4. The UIProcess calls the completion handler for the
`WebPageProxy::ExecuteUndoRedo` sync IPC,
thus finally responding to the WebProcess.
5. The WebProcess processes the `WebPage::UnapplyEditCommand` /
`WebPage::ReapplyEditCommand`
IPC
6. The WebProcess processes the reply to its `WebPageProxy::ExecuteUndoRedo`
sync IPC and returns
from its `sendSync()` call.
This is what normally happens. However, due to a race in our logic for handling
sync IPC responses,
steps 5 & 6 would sometimes be swapped, leading to crashes.
The race was due to the logic in Connection::waitForSyncReply(), which runs on
the main thread.
We would do the following:
1. Call `m_syncState->dispatchMessages()` to dispatch sync messages and async
messages sent with
the DispatchMessageEvenWhenWaitingForSyncReply option
2. Check if we received a response to our sync IPC, return it if we do
3. Go back to step 1
This code is racy because messages are processed on the IPC receive queue and
this code runs on the
main thread. This means that in between step 1 and step 2, we could receive
messages that should
have been processed *before* processing the sync IPC reply.
To address the issue, whenever we receive a sync IPC reply on the IPC receive
queue, we save (with
the reply) the identifiers of the IPC messages that were received *before* the
sync reply and that
should be processed before to maintain ordering. When we process the sync reply
later on, on the main
thread, we make sure to dispatch these messages *before* we return the sync
reply.
This patch also fixes a bug in SyncMessageState::dispatchMessages() that could
cause ordering issues.
The code wanted to merge 2 Deque containers and kept calling takeLast() on the
second container
instead of takeFirst(). This was essentially reversing the order of messages in
the second Deque.
* Source/WebKit/Platform/IPC/Connection.cpp:
(IPC::Connection::SyncMessageState::dispatchMessages):
(IPC::Connection::SyncMessageState::identifiersOfMessagesToDispatchWhileWaitingForSyncReply):
(IPC::Connection::waitForSyncReply):
(IPC::Connection::processIncomingSyncReply):
Canonical link: https://commits.webkit.org/274313.357@webkitglib/2.44
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
