Diff
Modified: trunk/Source/WebKit/ChangeLog (258336 => 258337)
--- trunk/Source/WebKit/ChangeLog 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/ChangeLog 2020-03-12 17:10:45 UTC (rev 258337)
@@ -1,3 +1,40 @@
+2020-03-12 Chris Dumez <[email protected]>
+
+ Networking process should kill the WebContent process if an invalid IPC message is received from it
+ https://bugs.webkit.org/show_bug.cgi?id=208999
+
+ Reviewed by Geoffrey Garen.
+
+ If the NetworkProcess receives a bad IPC from a WebProcess, it now sends an IPC to the UIProcess
+ asking for said WebProcess to be terminated.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::didReceiveInvalidMessage):
+
+ * Platform/IPC/HandleMessage.h:
+ (IPC::handleMessage):
+ (IPC::handleMessageSynchronous):
+ (IPC::handleMessageSynchronousWantsConnection):
+ (IPC::handleMessageAsync):
+ I noticed when testing this patch that the decoder was sometimes not marked as invalid even though
+ decoding failed (verified this by not decoding enough data or decoding too much data). As a result,
+ the IPC message would get ignored but didReceiveInvalidMessage() would not get called. To address
+ this, I know mark the decoder as invalid anytime decoding fails, instead of asserting that it is
+ already invalid.
+
+ * Shared/ProcessTerminationReason.h:
+ * UIProcess/API/C/WKAPICast.h:
+ (WebKit::toAPI):
+ * UIProcess/Cocoa/NavigationState.mm:
+ (WebKit::wkProcessTerminationReason):
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::terminateWebProcess):
+ * UIProcess/Network/NetworkProcessProxy.h:
+ * UIProcess/Network/NetworkProcessProxy.messages.in:
+ * UIProcess/WebPageProxy.cpp:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::requestTermination):
+
2020-03-12 Alex Christensen <[email protected]>
Remove unused GetWebCoreStatistics message
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (258336 => 258337)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2020-03-12 17:10:45 UTC (rev 258337)
@@ -337,8 +337,10 @@
#endif
}
-void NetworkConnectionToWebProcess::didReceiveInvalidMessage(IPC::Connection&, IPC::StringReference, IPC::StringReference)
+void NetworkConnectionToWebProcess::didReceiveInvalidMessage(IPC::Connection&, IPC::StringReference messageReceiverName, IPC::StringReference messageName)
{
+ RELEASE_LOG_FAULT(IPC, "Received an invalid message '%" PUBLIC_LOG_STRING "::%" PUBLIC_LOG_STRING "' from WebContent process %" PRIu64 ", requesting for it to be terminated.", messageReceiverName.toString().data(), messageName.toString().data(), m_webProcessIdentifier.toUInt64());
+ m_networkProcess->parentProcessConnection()->send(Messages::NetworkProcessProxy::TerminateWebProcess(m_webProcessIdentifier), 0);
}
void NetworkConnectionToWebProcess::createSocketStream(URL&& url, String cachePartition, WebSocketIdentifier identifier)
Modified: trunk/Source/WebKit/Platform/IPC/HandleMessage.h (258336 => 258337)
--- trunk/Source/WebKit/Platform/IPC/HandleMessage.h 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/Platform/IPC/HandleMessage.h 2020-03-12 17:10:45 UTC (rev 258337)
@@ -107,7 +107,7 @@
Optional<typename CodingType<typename T::Arguments>::Type> arguments;
decoder >> arguments;
if (!arguments) {
- ASSERT(decoder.isInvalid());
+ decoder.markInvalid();
return;
}
@@ -120,7 +120,7 @@
Optional<typename CodingType<typename T::Arguments>::Type> arguments;
decoder >> arguments;
if (!arguments) {
- ASSERT(decoder.isInvalid());
+ decoder.markInvalid();
return;
}
callMemberFunction(connection, WTFMove(*arguments), object, function);
@@ -132,7 +132,7 @@
Optional<typename CodingType<typename T::Arguments>::Type> arguments;
decoder >> arguments;
if (!arguments) {
- ASSERT(decoder.isInvalid());
+ decoder.markInvalid();
return;
}
@@ -148,7 +148,7 @@
Optional<typename CodingType<typename T::Arguments>::Type> arguments;
decoder >> arguments;
if (!arguments) {
- ASSERT(decoder.isInvalid());
+ decoder.markInvalid();
return;
}
@@ -164,7 +164,7 @@
Optional<uint64_t> listenerID;
decoder >> listenerID;
if (!listenerID) {
- ASSERT(decoder.isInvalid());
+ decoder.markInvalid();
return;
}
@@ -171,7 +171,7 @@
Optional<typename CodingType<typename T::Arguments>::Type> arguments;
decoder >> arguments;
if (!arguments) {
- ASSERT(decoder.isInvalid());
+ decoder.markInvalid();
return;
}
Modified: trunk/Source/WebKit/Shared/ProcessTerminationReason.h (258336 => 258337)
--- trunk/Source/WebKit/Shared/ProcessTerminationReason.h 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/Shared/ProcessTerminationReason.h 2020-03-12 17:10:45 UTC (rev 258337)
@@ -33,6 +33,7 @@
RequestedByClient,
Crash,
NavigationSwap,
+ RequestedByNetworkProcess,
};
}
Modified: trunk/Source/WebKit/UIProcess/API/C/WKAPICast.h (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/API/C/WKAPICast.h 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/API/C/WKAPICast.h 2020-03-12 17:10:45 UTC (rev 258337)
@@ -248,6 +248,7 @@
FALLTHROUGH;
case ProcessTerminationReason::RequestedByClient:
return kWKProcessTerminationReasonRequestedByClient;
+ case ProcessTerminationReason::RequestedByNetworkProcess:
case ProcessTerminationReason::Crash:
return kWKProcessTerminationReasonCrash;
}
Modified: trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm 2020-03-12 17:10:45 UTC (rev 258337)
@@ -1018,6 +1018,7 @@
FALLTHROUGH;
case ProcessTerminationReason::RequestedByClient:
return _WKProcessTerminationReasonRequestedByClient;
+ case ProcessTerminationReason::RequestedByNetworkProcess:
case ProcessTerminationReason::Crash:
return _WKProcessTerminationReasonCrash;
}
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2020-03-12 17:10:45 UTC (rev 258337)
@@ -439,6 +439,12 @@
page->logDiagnosticMessage(message, description, shouldSample);
}
+void NetworkProcessProxy::terminateWebProcess(WebCore::ProcessIdentifier webProcessIdentifier)
+{
+ if (auto* process = WebProcessProxy::processForIdentifier(webProcessIdentifier))
+ process->requestTermination(ProcessTerminationReason::RequestedByNetworkProcess);
+}
+
void NetworkProcessProxy::terminateUnresponsiveServiceWorkerProcesses(WebCore::ProcessIdentifier processIdentifier)
{
if (auto* process = WebProcessProxy::processForIdentifier(processIdentifier)) {
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2020-03-12 17:10:45 UTC (rev 258337)
@@ -277,6 +277,8 @@
void unregisterServiceWorkerClientProcess(WebCore::ProcessIdentifier webProcessIdentifier, WebCore::ProcessIdentifier serviceWorkerProcessIdentifier);
#endif
+ void terminateWebProcess(WebCore::ProcessIdentifier);
+
void requestStorageSpace(PAL::SessionID, const WebCore::ClientOrigin&, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t> quota)>&&);
WebsiteDataStore* websiteDataStoreFromSessionID(PAL::SessionID);
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in 2020-03-12 17:10:45 UTC (rev 258337)
@@ -57,6 +57,8 @@
RetrieveCacheStorageParameters(PAL::SessionID sessionID)
+ TerminateWebProcess(WebCore::ProcessIdentifier webProcessIdentifier)
+
#if ENABLE(SERVICE_WORKER)
EstablishWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain registrableDomain, PAL::SessionID sessionID) -> () Async
WorkerContextConnectionNoLongerNeeded(WebCore::ProcessIdentifier identifier)
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-03-12 17:10:45 UTC (rev 258337)
@@ -7306,6 +7306,7 @@
switch (reason) {
case ProcessTerminationReason::ExceededMemoryLimit:
case ProcessTerminationReason::ExceededCPULimit:
+ case ProcessTerminationReason::RequestedByNetworkProcess:
case ProcessTerminationReason::Crash:
return true;
case ProcessTerminationReason::NavigationSwap:
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (258336 => 258337)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2020-03-12 17:09:34 UTC (rev 258336)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2020-03-12 17:10:45 UTC (rev 258337)
@@ -1150,7 +1150,7 @@
return;
auto protectedThis = makeRef(*this);
- RELEASE_LOG_IF(isReleaseLoggingAllowed(), Process, "%p - WebProcessProxy::requestTermination - reason %d", this, reason);
+ RELEASE_LOG_ERROR_IF(isReleaseLoggingAllowed(), Process, "%p - WebProcessProxy::requestTermination - PID %d - reason %d", this, processIdentifier(), reason);
AuxiliaryProcessProxy::terminate();
