Changeset 263695 in webkit

Timestamp:

Jun 29, 2020 2:39:07 PM (4 years ago)

Author:

ddkilzer@apple.com

Message:

Crash in NetworkProcessProxy::getNetworkProcessConnection() lambda due to missing Optional<> value check
<​https://webkit.org/b/213700>
<rdar://problem/64852903>

Reviewed by Darin Adler.

• UIProcess/GPU/GPUProcessProxy.cpp:

(WebKit::GPUProcessProxy::getGPUProcessConnection):

• Rename connectionIdentifier to identifier.
• Return early from the lamba if identifier does not contain a value.
• UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::getNetworkProcessConnection):

• Ditto.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/GPU/GPUProcessProxy.cpp (modified) (1 diff)
• UIProcess/Network/NetworkProcessProxy.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-06-29  David Kilzer  <ddkilzer@apple.com>
+
+        Crash in NetworkProcessProxy::getNetworkProcessConnection() lambda due to missing Optional<> value check
+        <https://webkit.org/b/213700>
+        <rdar://problem/64852903>
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/GPU/GPUProcessProxy.cpp:
+        (WebKit::GPUProcessProxy::getGPUProcessConnection):
+        - Rename `connectionIdentifier` to `identifier`.
+        - Return early from the lamba if `identifier` does not contain a
+          value.
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::getNetworkProcessConnection):
+        - Ditto.
+
 2020-06-29  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp

@@ -184 +184 @@
 
     RELEASE_LOG(ProcessSuspension, "%p - GPUProcessProxy is taking a background assertion because a web process is requesting a connection", this);
-    sendWithAsyncReply(Messages::GPUProcess::CreateGPUConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply)](auto&& connectionIdentifier) mutable {
+    sendWithAsyncReply(Messages::GPUProcess::CreateGPUConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply)](auto&& identifier) mutable {
         if (!weakThis) {
             RELEASE_LOG_ERROR(Process, "GPUProcessProxy::getGPUProcessConnection: GPUProcessProxy deallocated during connection establishment");
             return reply({ });
         }
+
+        if (!identifier) {
+            RELEASE_LOG_ERROR(Process, "GPUProcessProxy::getGPUProcessConnection: connection identifier is empty");
+            return reply({ });
+        }
+
 #if USE(UNIX_DOMAIN_SOCKETS) || OS(WINDOWS)
-        reply(GPUProcessConnectionInfo { WTFMove(*connectionIdentifier) });
+        reply(GPUProcessConnectionInfo { WTFMove(*identifier) });
         UNUSED_VARIABLE(this);
 #elif OS(DARWIN)
-        MESSAGE_CHECK(MACH_PORT_VALID(connectionIdentifier->port()));
-        reply(GPUProcessConnectionInfo { IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, this->connection()->getAuditToken() });
+        MESSAGE_CHECK(MACH_PORT_VALID(identifier->port()));
+        reply(GPUProcessConnectionInfo { IPC::Attachment { identifier->port(), MACH_MSG_TYPE_MOVE_SEND }, this->connection()->getAuditToken() });
 #else
         notImplemented();

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -134 +134 @@
 {
     RELEASE_LOG(ProcessSuspension, "%p - NetworkProcessProxy is taking a background assertion because a web process is requesting a connection", this);
-    sendWithAsyncReply(Messages::NetworkProcess::CreateNetworkConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply)](auto&& connectionIdentifier, auto cookieAcceptPolicy) mutable {
+    sendWithAsyncReply(Messages::NetworkProcess::CreateNetworkConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply)](auto&& identifier, auto cookieAcceptPolicy) mutable {
         if (!weakThis) {
             RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::getNetworkProcessConnection: NetworkProcessProxy deallocated during connection establishment");
@@ -140 +140 @@
         }
 
+        if (!identifier) {
+            RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::getNetworkProcessConnection: connection identifier is empty");
+            return reply({ });
+        }
+
 #if USE(UNIX_DOMAIN_SOCKETS) || OS(WINDOWS)
-        reply(NetworkProcessConnectionInfo { WTFMove(*connectionIdentifier), cookieAcceptPolicy });
+        reply(NetworkProcessConnectionInfo { WTFMove(*identifier), cookieAcceptPolicy });
         UNUSED_VARIABLE(this);
 #elif OS(DARWIN)
-        MESSAGE_CHECK(MACH_PORT_VALID(connectionIdentifier->port()));
-        reply(NetworkProcessConnectionInfo { IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, cookieAcceptPolicy, connection()->getAuditToken() });
+        MESSAGE_CHECK(MACH_PORT_VALID(identifier->port()));
+        reply(NetworkProcessConnectionInfo { IPC::Attachment { identifier->port(), MACH_MSG_TYPE_MOVE_SEND }, cookieAcceptPolicy, connection()->getAuditToken() });
 #else
         notImplemented();