- Revision
- 264846
- Author
- [email protected]
- Date
- 2020-07-24 11:41:28 -0700 (Fri, 24 Jul 2020)
Log Message
Crash under PreconnectTask::start()
https://bugs.webkit.org/show_bug.cgi?id=214747
<rdar://problem/65989668>
Reviewed by Geoffrey Garen.
All user of PreconnectTask construct a PreconnectTask and then call start() on it.
start() dereferences m_networkLoad unconditionally. However, the PreconnectTask
constructor may return early and not initialize m_networkLoad if there is not
NetworkSession for the given sessionID.
To address the issue, update call sites to pass in a valid NetworkSession, so that
the PreconnectTask now always initializes m_networkLoad.
No new tests, we do not know how to reproduce, likely due to a race between
a preconnect request being issued and a network session getting destroyed.
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::sendH2Ping):
(WebKit::NetworkConnectionToWebProcess::preconnectTo):
* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::preconnectTo):
* NetworkProcess/PreconnectTask.cpp:
(WebKit::PreconnectTask::PreconnectTask):
* NetworkProcess/PreconnectTask.h:
* NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
(WebKit::NetworkCache::SpeculativeLoadManager::preconnectForSubresource):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (264845 => 264846)
--- trunk/Source/WebKit/ChangeLog 2020-07-24 18:33:15 UTC (rev 264845)
+++ trunk/Source/WebKit/ChangeLog 2020-07-24 18:41:28 UTC (rev 264846)
@@ -1,3 +1,33 @@
+2020-07-24 Chris Dumez <[email protected]>
+
+ Crash under PreconnectTask::start()
+ https://bugs.webkit.org/show_bug.cgi?id=214747
+ <rdar://problem/65989668>
+
+ Reviewed by Geoffrey Garen.
+
+ All user of PreconnectTask construct a PreconnectTask and then call start() on it.
+ start() dereferences m_networkLoad unconditionally. However, the PreconnectTask
+ constructor may return early and not initialize m_networkLoad if there is not
+ NetworkSession for the given sessionID.
+
+ To address the issue, update call sites to pass in a valid NetworkSession, so that
+ the PreconnectTask now always initializes m_networkLoad.
+
+ No new tests, we do not know how to reproduce, likely due to a race between
+ a preconnect request being issued and a network session getting destroyed.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::sendH2Ping):
+ (WebKit::NetworkConnectionToWebProcess::preconnectTo):
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::preconnectTo):
+ * NetworkProcess/PreconnectTask.cpp:
+ (WebKit::PreconnectTask::PreconnectTask):
+ * NetworkProcess/PreconnectTask.h:
+ * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+ (WebKit::NetworkCache::SpeculativeLoadManager::preconnectForSubresource):
+
2020-07-24 David Kilzer <[email protected]>
[IPC hardening] Add nullptr checks for WebPage::m_activeColorChooser
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (264845 => 264846)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2020-07-24 18:33:15 UTC (rev 264845)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2020-07-24 18:41:28 UTC (rev 264846)
@@ -550,8 +550,12 @@
void NetworkConnectionToWebProcess::sendH2Ping(NetworkResourceLoadParameters&& parameters, CompletionHandler<void(Expected<Seconds, ResourceError>&&)>&& completionHandler)
{
#if ENABLE(SERVER_PRECONNECT)
+ auto* networkSession = this->networkSession();
+ if (!networkSession)
+ return completionHandler(makeUnexpected(internalError(parameters.request.url())));
+
URL url = ""
- auto* task = new PreconnectTask(networkProcess(), sessionID(), WTFMove(parameters), [] (const ResourceError&) { });
+ auto* task = new PreconnectTask(*networkSession, WTFMove(parameters), [] (const ResourceError&) { });
task->setH2PingCallback(url, WTFMove(completionHandler));
task->start();
#else
@@ -581,7 +585,7 @@
#if ENABLE(SERVER_PRECONNECT)
auto* session = networkSession();
if (session && session->allowsServerPreconnect()) {
- (new PreconnectTask(networkProcess(), sessionID(), WTFMove(loadParameters), [completionHandler = WTFMove(completionHandler)] (const ResourceError& error) {
+ (new PreconnectTask(*session, WTFMove(loadParameters), [completionHandler = WTFMove(completionHandler)] (const ResourceError& error) {
completionHandler(error);
}))->start();
return;
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (264845 => 264846)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-07-24 18:33:15 UTC (rev 264845)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-07-24 18:41:28 UTC (rev 264846)
@@ -1404,6 +1404,10 @@
return;
#endif
+ auto* networkSession = this->networkSession(sessionID);
+ if (!networkSession)
+ return;
+
NetworkLoadParameters parameters;
parameters.request = ResourceRequest { url };
parameters.webPageProxyID = webPageProxyID;
@@ -1417,7 +1421,7 @@
parameters.storedCredentialsPolicy = storedCredentialsPolicy;
parameters.shouldPreconnectOnly = PreconnectOnly::Yes;
- (new PreconnectTask(*this, sessionID, WTFMove(parameters), [](const WebCore::ResourceError&) { }))->start();
+ (new PreconnectTask(*networkSession, WTFMove(parameters), [](const WebCore::ResourceError&) { }))->start();
#else
UNUSED_PARAM(url);
UNUSED_PARAM(userAgent);
Modified: trunk/Source/WebKit/NetworkProcess/PreconnectTask.cpp (264845 => 264846)
--- trunk/Source/WebKit/NetworkProcess/PreconnectTask.cpp 2020-07-24 18:33:15 UTC (rev 264845)
+++ trunk/Source/WebKit/NetworkProcess/PreconnectTask.cpp 2020-07-24 18:41:28 UTC (rev 264846)
@@ -39,21 +39,14 @@
using namespace WebCore;
-PreconnectTask::PreconnectTask(NetworkProcess& networkProcess, PAL::SessionID sessionID, NetworkLoadParameters&& parameters, CompletionHandler<void(const ResourceError&)>&& completionHandler)
+PreconnectTask::PreconnectTask(NetworkSession& networkSession, NetworkLoadParameters&& parameters, CompletionHandler<void(const ResourceError&)>&& completionHandler)
: m_completionHandler(WTFMove(completionHandler))
, m_timeoutTimer([this] { didFinish(ResourceError { String(), 0, m_networkLoad->parameters().request.url(), "Preconnection timed out"_s, ResourceError::Type::Timeout }); })
{
RELEASE_LOG(Network, "%p - PreconnectTask::PreconnectTask()", this);
- auto* networkSession = networkProcess.networkSession(sessionID);
- if (!networkSession) {
- ASSERT_NOT_REACHED();
- m_completionHandler(internalError(parameters.request.url()));
- return;
- }
-
ASSERT(parameters.shouldPreconnectOnly == PreconnectOnly::Yes);
- m_networkLoad = makeUnique<NetworkLoad>(*this, nullptr, WTFMove(parameters), *networkSession);
+ m_networkLoad = makeUnique<NetworkLoad>(*this, nullptr, WTFMove(parameters), networkSession);
m_timeoutTimer.startOneShot(60000_s);
}
Modified: trunk/Source/WebKit/NetworkProcess/PreconnectTask.h (264845 => 264846)
--- trunk/Source/WebKit/NetworkProcess/PreconnectTask.h 2020-07-24 18:33:15 UTC (rev 264845)
+++ trunk/Source/WebKit/NetworkProcess/PreconnectTask.h 2020-07-24 18:41:28 UTC (rev 264846)
@@ -40,7 +40,7 @@
class PreconnectTask final : public NetworkLoadClient {
public:
- PreconnectTask(NetworkProcess&, PAL::SessionID, NetworkLoadParameters&&, CompletionHandler<void(const WebCore::ResourceError&)>&&);
+ PreconnectTask(NetworkSession&, NetworkLoadParameters&&, CompletionHandler<void(const WebCore::ResourceError&)>&&);
~PreconnectTask();
void setH2PingCallback(const URL&, CompletionHandler<void(Expected<WTF::Seconds, WebCore::ResourceError>&&)>&&);
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (264845 => 264846)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp 2020-07-24 18:33:15 UTC (rev 264845)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp 2020-07-24 18:41:28 UTC (rev 264846)
@@ -453,6 +453,10 @@
void SpeculativeLoadManager::preconnectForSubresource(const SubresourceInfo& subresourceInfo, Entry* entry, const GlobalFrameID& frameID, Optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain)
{
#if ENABLE(SERVER_PRECONNECT)
+ auto* networkSession = m_cache.networkProcess().networkSession(m_cache.sessionID());
+ if (!networkSession)
+ return;
+
NetworkLoadParameters parameters;
parameters.webPageProxyID = frameID.webPageProxyID;
parameters.webPageID = frameID.webPageID;
@@ -463,7 +467,7 @@
parameters.shouldPreconnectOnly = PreconnectOnly::Yes;
parameters.request = constructRevalidationRequest(subresourceInfo.key(), subresourceInfo, entry);
parameters.isNavigatingToAppBoundDomain = isNavigatingToAppBoundDomain;
- (new PreconnectTask(m_cache.networkProcess(), m_cache.sessionID(), WTFMove(parameters), [](const WebCore::ResourceError&) { }))->start();
+ (new PreconnectTask(*networkSession, WTFMove(parameters), [](const WebCore::ResourceError&) { }))->start();
#else
UNUSED_PARAM(subresourceInfo);
UNUSED_PARAM(entry);
