Skip to content

Commit

Permalink
Cherry-pick 259548.751@safari-7615-branch (9fc6529). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=256373

    Drop MessageName::Terminate IPC message
    https://bugs.webkit.org/show_bug.cgi?id=256373
    rdar://108899132

    Reviewed by Sihui Liu.

    Drop MessageName::Terminate IPC message as it could easily be abused by a
    compromised WebProcess to kill the UIProcess.

    It was temporarily introduced in 243810@main to investigate NetworkProcess
    hangs we were seeing and should no longer be needed. This reverts 243810@main.

    * Source/WebKit/Platform/IPC/Connection.cpp:
    (IPC::Connection::processIncomingMessage):
    (IPC::terminateDueToIPCTerminateMessage): Deleted.
    * Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:
    (WebKit::NetworkProcessProxy::didBecomeUnresponsive):
    (WebKit::shouldTerminateNetworkProcessBySendingMessage): Deleted.

    Canonical link: https://commits.webkit.org/259548.751@safari-7615-branch
  • Loading branch information
cdumez authored and mcatanzaro committed Jul 28, 2023
1 parent 8c36278 commit 818c5df
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 53 deletions.
14 changes: 0 additions & 14 deletions Source/WebKit/Platform/IPC/Connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@

#if PLATFORM(COCOA)
#include "MachMessage.h"
#include "WKCrashReporter.h"
#endif

#if USE(UNIX_DOMAIN_SOCKETS)
Expand Down Expand Up @@ -835,16 +834,6 @@ void Connection::processIncomingSyncReply(std::unique_ptr<Decoder> decoder)
// This can happen if the send timed out, so it's fine to ignore.
}

static NEVER_INLINE NO_RETURN_DUE_TO_CRASH void terminateDueToIPCTerminateMessage()
{
#if PLATFORM(COCOA)
WebKit::logAndSetCrashLogMessage("Receives Terminate message");
#else
WTFLogAlways("Receives Terminate message");
#endif
CRASH();
}

void Connection::processIncomingMessage(std::unique_ptr<Decoder> message)
{
ASSERT(message->messageReceiverName() != ReceiverName::Invalid);
Expand All @@ -854,9 +843,6 @@ void Connection::processIncomingMessage(std::unique_ptr<Decoder> message)
return;
}

if (message->messageName() == MessageName::Terminate)
return terminateDueToIPCTerminateMessage();

if (!MessageReceiveQueueMap::isValidMessage(*message)) {
dispatchDidReceiveInvalidMessage(message->messageName());
return;
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/Scripts/webkit/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ def has_attribute(self, attribute):
Message('LegacySessionState', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
Message('SetStreamDestinationID', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
Message('ProcessOutOfStreamMessage', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
Message('Terminate', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
], condition=None)


Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/Scripts/webkit/tests/MessageNames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ const MessageDescription messageDescriptions[static_cast<size_t>(MessageName::Co
{ "ProcessOutOfStreamMessage", ReceiverName::IPC, false, false },
{ "SetStreamDestinationID", ReceiverName::IPC, false, false },
{ "SyncMessageReply", ReceiverName::IPC, false, false },
{ "Terminate", ReceiverName::IPC, false, false },
#if USE(AVFOUNDATION)
{ "TestWithCVPixelBuffer_ReceiveCVPixelBufferReply", ReceiverName::AsyncReply, false, false },
#endif
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/Scripts/webkit/tests/MessageNames.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ enum class MessageName : uint16_t {
ProcessOutOfStreamMessage,
SetStreamDestinationID,
SyncMessageReply,
Terminate,
#if USE(AVFOUNDATION)
TestWithCVPixelBuffer_ReceiveCVPixelBufferReply,
#endif
Expand Down
36 changes: 0 additions & 36 deletions Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ namespace WebKit {
using namespace WebCore;

static constexpr Seconds networkProcessResponsivenessTimeout = 6_s;
static constexpr int unresponsivenessCountLimit = 3;
static constexpr Seconds unresponsivenessCheckPeriod = 15_s;

static HashSet<NetworkProcessProxy*>& networkProcessesSet()
{
Expand All @@ -107,25 +105,6 @@ static HashSet<NetworkProcessProxy*>& networkProcessesSet()
return set;
}

static bool shouldTerminateNetworkProcessBySendingMessage()
{
static WallTime unresponsivenessPeriodStartTime = WallTime::now();
static int unresponsivenessCountDuringThisPeriod = 0;
auto now = WallTime::now();

if (now - unresponsivenessPeriodStartTime > unresponsivenessCheckPeriod) {
unresponsivenessCountDuringThisPeriod = 1;
unresponsivenessPeriodStartTime = now;
return false;
}

++unresponsivenessCountDuringThisPeriod;
if (unresponsivenessCountDuringThisPeriod >= unresponsivenessCountLimit)
return true;

return false;
}

Vector<Ref<NetworkProcessProxy>> NetworkProcessProxy::allNetworkProcesses()
{
return WTF::map(networkProcessesSet(), [](auto* networkProcess) {
Expand Down Expand Up @@ -167,21 +146,6 @@ void NetworkProcessProxy::didBecomeUnresponsive()
{
RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::didBecomeUnresponsive: NetworkProcess with PID %d became unresponsive, terminating it", processIdentifier());

// Let network process terminates itself and generate crash report for investigation of hangs.
// We currently only do this when network process becomes unresponsive multiple times in a short
// time period to avoid generating too many crash reports with same back trace on user's device.
if (shouldTerminateNetworkProcessBySendingMessage()) {
sendMessage(makeUniqueRef<IPC::Encoder>(IPC::MessageName::Terminate, 0), { });
RunLoop::main().dispatchAfter(1_s, [weakThis = WeakPtr { *this }] () mutable {
if (weakThis) {
weakThis->terminate();
weakThis->networkProcessDidTerminate(ProcessTerminationReason::Unresponsive);
}

});
return;
}

terminate();
networkProcessDidTerminate(ProcessTerminationReason::Unresponsive);
}
Expand Down

0 comments on commit 818c5df

Please sign in to comment.