Title: [233815] trunk/Source/WebKit
- Revision
- 233815
- Author
- [email protected]
- Date
- 2018-07-13 13:40:45 -0700 (Fri, 13 Jul 2018)
Log Message
WebResourceLoader may try to send a IPC with a destination ID that is 0
https://bugs.webkit.org/show_bug.cgi?id=187654
<rdar://problem/39265927>
Reviewed by Brady Eidson.
WebResourceLoader may try to send a IPC with a destination ID that is 0 according to release
assertion hits we see in the wild. This can lead to HashMap corruption on recipient side when
trying to lookup a key that is 0.
As per the crash traces, we see it starts from WebLoaderStrategy::internallyFailedLoadTimerFired()
which is likely due to the Network process crashing. WebLoaderStrategy::networkProcessCrashed()
calls scheduleInternallyFailedLoad() on each ResourceLoader and clears the WebResourceLoaders in
m_webResourceLoaders. When a ResourceLoader is cancelled (marked as failed), we clear its identifier
and we normally null out the WebLoaderStrategy's coreLoader. However, if we cannot find the
WebResourceLoader in m_webResourceLoaders (because that map was already cleared) then the
WebResourceLoader's coreLoader may not get nulled out, even if the coreLoader's identifier has
been reset. We have 2 lambdas in WebResourceLoader which keep the WebResourceLoader alive and
try to send IPC and are merely doing a null check on the coreLoader before trying to send the
IPC.
To address the issue, we now clear the WebResourceLoader's coreLoader in
WebLoaderStrategy::networkProcessCrashed(). For robustness, we also check that a coreLoader's
identifier is not 0 before trying to send IPC.
* WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::networkProcessCrashed):
* WebProcess/Network/WebResourceLoader.cpp:
(WebKit::WebResourceLoader::willSendRequest):
(WebKit::WebResourceLoader::didReceiveResponse):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (233814 => 233815)
--- trunk/Source/WebKit/ChangeLog 2018-07-13 20:34:24 UTC (rev 233814)
+++ trunk/Source/WebKit/ChangeLog 2018-07-13 20:40:45 UTC (rev 233815)
@@ -1,3 +1,36 @@
+2018-07-13 Chris Dumez <[email protected]>
+
+ WebResourceLoader may try to send a IPC with a destination ID that is 0
+ https://bugs.webkit.org/show_bug.cgi?id=187654
+ <rdar://problem/39265927>
+
+ Reviewed by Brady Eidson.
+
+ WebResourceLoader may try to send a IPC with a destination ID that is 0 according to release
+ assertion hits we see in the wild. This can lead to HashMap corruption on recipient side when
+ trying to lookup a key that is 0.
+
+ As per the crash traces, we see it starts from WebLoaderStrategy::internallyFailedLoadTimerFired()
+ which is likely due to the Network process crashing. WebLoaderStrategy::networkProcessCrashed()
+ calls scheduleInternallyFailedLoad() on each ResourceLoader and clears the WebResourceLoaders in
+ m_webResourceLoaders. When a ResourceLoader is cancelled (marked as failed), we clear its identifier
+ and we normally null out the WebLoaderStrategy's coreLoader. However, if we cannot find the
+ WebResourceLoader in m_webResourceLoaders (because that map was already cleared) then the
+ WebResourceLoader's coreLoader may not get nulled out, even if the coreLoader's identifier has
+ been reset. We have 2 lambdas in WebResourceLoader which keep the WebResourceLoader alive and
+ try to send IPC and are merely doing a null check on the coreLoader before trying to send the
+ IPC.
+
+ To address the issue, we now clear the WebResourceLoader's coreLoader in
+ WebLoaderStrategy::networkProcessCrashed(). For robustness, we also check that a coreLoader's
+ identifier is not 0 before trying to send IPC.
+
+ * WebProcess/Network/WebLoaderStrategy.cpp:
+ (WebKit::WebLoaderStrategy::networkProcessCrashed):
+ * WebProcess/Network/WebResourceLoader.cpp:
+ (WebKit::WebResourceLoader::willSendRequest):
+ (WebKit::WebResourceLoader::didReceiveResponse):
+
2018-07-13 Zach Li <[email protected]>
Allow BOCOM and ABC plug-ins to run unsandboxed
Modified: trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp (233814 => 233815)
--- trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp 2018-07-13 20:34:24 UTC (rev 233814)
+++ trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp 2018-07-13 20:40:45 UTC (rev 233815)
@@ -450,8 +450,10 @@
{
RELEASE_LOG_ERROR(Network, "WebLoaderStrategy::networkProcessCrashed: failing all pending resource loaders");
- for (auto& loader : m_webResourceLoaders)
- scheduleInternallyFailedLoad(*loader.value->resourceLoader());
+ for (auto& loader : m_webResourceLoaders.values()) {
+ scheduleInternallyFailedLoad(*loader->resourceLoader());
+ loader->detachFromCoreLoader();
+ }
m_webResourceLoaders.clear();
Modified: trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp (233814 => 233815)
--- trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp 2018-07-13 20:34:24 UTC (rev 233814)
+++ trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp 2018-07-13 20:40:45 UTC (rev 233815)
@@ -92,11 +92,11 @@
if (m_coreLoader->documentLoader()->applicationCacheHost().maybeLoadFallbackForRedirect(m_coreLoader.get(), proposedRequest, redirectResponse))
return;
- m_coreLoader->willSendRequest(WTFMove(proposedRequest), redirectResponse, [protectedThis = makeRef(*this)](ResourceRequest&& request) {
- if (!protectedThis->m_coreLoader)
+ m_coreLoader->willSendRequest(WTFMove(proposedRequest), redirectResponse, [this, protectedThis = makeRef(*this)](ResourceRequest&& request) {
+ if (!m_coreLoader || !m_coreLoader->identifier())
return;
- protectedThis->send(Messages::NetworkResourceLoader::ContinueWillSendRequest(request, protectedThis->m_coreLoader->isAllowedToAskUserForCredentials()));
+ send(Messages::NetworkResourceLoader::ContinueWillSendRequest(request, m_coreLoader->isAllowedToAskUserForCredentials()));
});
}
@@ -125,7 +125,7 @@
m_isProcessingNetworkResponse = false;
#endif
// If m_coreLoader becomes null as a result of the didReceiveResponse callback, we can't use the send function().
- if (m_coreLoader)
+ if (m_coreLoader && m_coreLoader->identifier())
send(Messages::NetworkResourceLoader::ContinueDidReceiveResponse());
};
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
