From 4420d02e593d38c03d9ff8eac5466103d3b2077e Mon Sep 17 00:00:00 2001 From: Kimmo Kinnunen Date: Wed, 29 May 2024 04:39:31 -0700 Subject: [PATCH] Web process sends redundant information when establishing GPU process connection https://bugs.webkit.org/show_bug.cgi?id=274307 rdar://128265618 Reviewed by Chris Dumez. Reland: Use HAVE(TASK_IDENTITY_TOKEN) instead of PLATFORM(COCOA) to guard ASSERT(m_processIdentity), as the iOS Family simulators do not have the task identity token. Web process would send - the process identity for resource attribution - settings object The settings were previously changled to be filled by the privileged UI process. Remove this altogether, as it's insecure to let the WP select anything here. The process identity does not change once the WP is created. Thus send this during WP initialization. Moves the WebProcessProxy message sends to the WebProcessProxy. * Source/WebKit/Scripts/webkit/messages.py: (types_that_must_be_moved): * Source/WebKit/UIProcess/WebProcessPool.cpp: (WebKit::WebProcessPool::createGPUProcessConnection): * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::initializeIdentity): (WebKit::WebProcessProxy::createGPUProcessConnection): * Source/WebKit/UIProcess/WebProcessProxy.h: * Source/WebKit/UIProcess/WebProcessProxy.messages.in: * Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp: (WebKit::GPUProcessConnection::create): (WebKit::getGPUProcessConnectionParameters): Deleted. * Source/WebKit/WebProcess/WebProcess.cpp: (WebKit::WebProcess::initializeWebProcess): Canonical link: https://commits.webkit.org/279438@main --- Source/WebKit/Scripts/webkit/messages.py | 1 + Source/WebKit/UIProcess/WebProcessPool.cpp | 9 +----- Source/WebKit/UIProcess/WebProcessProxy.cpp | 21 ++++++++++++-- Source/WebKit/UIProcess/WebProcessProxy.h | 7 ++++- .../UIProcess/WebProcessProxy.messages.in | 2 +- .../WebProcess/GPU/GPUProcessConnection.cpp | 29 +++---------------- .../WebProcess/GPU/GPUProcessConnection.h | 4 +-- Source/WebKit/WebProcess/WebProcess.cpp | 15 ++++++++-- Source/WebKit/WebProcess/WebProcess.h | 2 +- .../WebKit/WebProcess/WebProcess.messages.in | 2 +- 10 files changed, 48 insertions(+), 44 deletions(-) diff --git a/Source/WebKit/Scripts/webkit/messages.py b/Source/WebKit/Scripts/webkit/messages.py index e0c1011ccfd34..08375bb3d6279 100644 --- a/Source/WebKit/Scripts/webkit/messages.py +++ b/Source/WebKit/Scripts/webkit/messages.py @@ -146,6 +146,7 @@ def types_that_must_be_moved(): 'WebKit::WebGPU::ExternalTextureDescriptor', 'WebCore::GraphicsContextGL::ExternalImageSource', 'WebCore::GraphicsContextGL::ExternalSyncSource', + 'WebCore::ProcessIdentity', 'WebKit::ConsumerSharedCARingBufferHandle', 'WebKit::GPUProcessConnectionParameters', 'WebKit::ModelProcessConnectionParameters', diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp index 53402c86c88b5..2e82b63f1ab66 100644 --- a/Source/WebKit/UIProcess/WebProcessPool.cpp +++ b/Source/WebKit/UIProcess/WebProcessPool.cpp @@ -555,16 +555,9 @@ void WebProcessPool::gpuProcessExited(ProcessID identifier, ProcessTerminationRe void WebProcessPool::createGPUProcessConnection(WebProcessProxy& webProcessProxy, IPC::Connection::Handle&& connectionIdentifier, WebKit::GPUProcessConnectionParameters&& parameters) { -#if ENABLE(IPC_TESTING_API) - parameters.ignoreInvalidMessageForTesting = webProcessProxy.ignoreInvalidMessageForTesting(); -#endif - #if HAVE(AUDIT_TOKEN) parameters.presentingApplicationAuditToken = configuration().presentingApplicationProcessToken(); #endif - - parameters.isLockdownModeEnabled = webProcessProxy.lockdownMode() == WebProcessProxy::LockdownMode::Enabled; - ensureProtectedGPUProcess()->createGPUProcessConnection(webProcessProxy, WTFMove(connectionIdentifier), WTFMove(parameters)); } #endif // ENABLE(GPU_PROCESS) @@ -1011,7 +1004,7 @@ void WebProcessPool::initializeNewWebProcess(WebProcessProxy& process, WebsiteDa if (websiteDataStore) parameters.websiteDataStoreParameters = webProcessDataStoreParameters(process, *websiteDataStore); - process.send(Messages::WebProcess::InitializeWebProcess(WTFMove(parameters)), 0); + process.initializeWebProcess(WTFMove(parameters)); #if HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK) setMediaAccessibilityPreferences(process); diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp index 0712d21717765..4c665d3fb87a3 100644 --- a/Source/WebKit/UIProcess/WebProcessProxy.cpp +++ b/Source/WebKit/UIProcess/WebProcessProxy.cpp @@ -32,6 +32,7 @@ #include "APIUIClient.h" #include "AuthenticatorManager.h" #include "DownloadProxyMap.h" +#include "GPUProcessConnectionParameters.h" #include "GoToBackForwardItemParameters.h" #include "LoadParameters.h" #include "Logging.h" @@ -463,6 +464,14 @@ void WebProcessProxy::updateRegistrationWithDataStore() } } +void WebProcessProxy::initializeWebProcess(WebProcessCreationParameters&& parameters) +{ + sendWithAsyncReply(Messages::WebProcess::InitializeWebProcess(WTFMove(parameters)), [weakThis = WeakPtr { *this }] (ProcessIdentity processIdentity) { + if (RefPtr protectedThis = weakThis.get()) + protectedThis->m_processIdentity = WTFMove(processIdentity); + }, 0); +} + void WebProcessProxy::initializePreferencesForGPUAndNetworkProcesses(const WebPageProxy& page) { #if ENABLE(GPU_PROCESS) @@ -1063,13 +1072,21 @@ void WebProcessProxy::getNetworkProcessConnection(CompletionHandlercreateGPUProcessConnection(*this, WTFMove(connectionIdentifier), WTFMove(parameters)); } diff --git a/Source/WebKit/UIProcess/WebProcessProxy.h b/Source/WebKit/UIProcess/WebProcessProxy.h index a0bc86087fe24..c9c6e68e6ab25 100644 --- a/Source/WebKit/UIProcess/WebProcessProxy.h +++ b/Source/WebKit/UIProcess/WebProcessProxy.h @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -91,6 +92,7 @@ class ResourceRequest; struct NotificationData; struct PluginInfo; struct PrewarmInformation; +struct WebProcessCreationParameters; class SecurityOriginData; enum class PermissionName : uint8_t; enum class ThirdPartyCookieBlockingMode : uint8_t; @@ -176,6 +178,8 @@ class WebProcessProxy : public AuxiliaryProcessProxy { static void forWebPagesWithOrigin(PAL::SessionID, const WebCore::SecurityOriginData&, const Function&); static Vector> allowedFirstPartiesForCookies(); + void initializeWebProcess(WebProcessCreationParameters&&); + WebConnection* webConnection() const { return m_webConnection.get(); } RefPtr protectedWebConnection() const { return m_webConnection; } @@ -575,7 +579,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy { void getNetworkProcessConnection(CompletionHandler&&); #if ENABLE(GPU_PROCESS) - void createGPUProcessConnection(IPC::Connection::Handle&&, WebKit::GPUProcessConnectionParameters&&); + void createGPUProcessConnection(IPC::Connection::Handle&&); #endif #if ENABLE(MODEL_PROCESS) @@ -792,6 +796,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy { Seconds m_totalForegroundTime; Seconds m_totalBackgroundTime; Seconds m_totalSuspendedTime; + WebCore::ProcessIdentity m_processIdentity; }; WTF::TextStream& operator<<(WTF::TextStream&, const WebProcessProxy&); diff --git a/Source/WebKit/UIProcess/WebProcessProxy.messages.in b/Source/WebKit/UIProcess/WebProcessProxy.messages.in index 83fa16d1a3392..1e4a0a514b12e 100644 --- a/Source/WebKit/UIProcess/WebProcessProxy.messages.in +++ b/Source/WebKit/UIProcess/WebProcessProxy.messages.in @@ -34,7 +34,7 @@ messages -> WebProcessProxy LegacyReceiver { GetNetworkProcessConnection() -> (struct WebKit::NetworkProcessConnectionInfo connectionInfo) Synchronous #if ENABLE(GPU_PROCESS) - CreateGPUProcessConnection(IPC::ConnectionHandle connectionHandle, struct WebKit::GPUProcessConnectionParameters parameters) AllowedWhenWaitingForSyncReply + CreateGPUProcessConnection(IPC::ConnectionHandle connectionHandle) AllowedWhenWaitingForSyncReply #endif #if ENABLE(MODEL_PROCESS) diff --git a/Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp b/Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp index 690f4946f501c..5d2c011c6fd9e 100644 --- a/Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp +++ b/Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp @@ -32,7 +32,6 @@ #include "GPUConnectionToWebProcessMessages.h" #include "GPUProcessConnectionInfo.h" #include "GPUProcessConnectionMessages.h" -#include "GPUProcessConnectionParameters.h" #include "LibWebRTCCodecs.h" #include "LibWebRTCCodecsMessages.h" #include "Logging.h" @@ -103,35 +102,15 @@ namespace WebKit { using namespace WebCore; -static GPUProcessConnectionParameters getGPUProcessConnectionParameters() +Ref GPUProcessConnection::create(Ref&& connection) { - GPUProcessConnectionParameters parameters; -#if PLATFORM(COCOA) - parameters.webProcessIdentity = ProcessIdentity { ProcessIdentity::CurrentProcess }; -#endif - return parameters; -} - -RefPtr GPUProcessConnection::create(IPC::Connection& parentConnection) -{ - auto connectionIdentifiers = IPC::Connection::createConnectionIdentifierPair(); - if (!connectionIdentifiers) - return nullptr; - - RELEASE_ASSERT_WITH_MESSAGE(WebProcess::singleton().hasEverHadAnyWebPages(), "GPUProcess preferences come from the pages"); - parentConnection.send(Messages::WebProcessProxy::CreateGPUProcessConnection(WTFMove(connectionIdentifiers->client), getGPUProcessConnectionParameters()), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply); - - auto instance = adoptRef(*new GPUProcessConnection(WTFMove(connectionIdentifiers->server))); -#if ENABLE(IPC_TESTING_API) - if (parentConnection.ignoreInvalidMessageForTesting()) - instance->connection().setIgnoreInvalidMessageForTesting(); -#endif + Ref instance = adoptRef(*new GPUProcessConnection(WTFMove(connection))); RELEASE_LOG(Process, "GPUProcessConnection::create - %p", instance.ptr()); return instance; } -GPUProcessConnection::GPUProcessConnection(IPC::Connection::Identifier&& connectionIdentifier) - : m_connection(IPC::Connection::createServerConnection(connectionIdentifier)) +GPUProcessConnection::GPUProcessConnection(Ref&& connection) + : m_connection(WTFMove(connection)) { m_connection->open(*this); diff --git a/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h b/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h index 63bff21c4efe1..26c53704081cd 100644 --- a/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h +++ b/Source/WebKit/WebProcess/GPU/GPUProcessConnection.h @@ -66,7 +66,7 @@ class RemoteVideoFrameObjectHeapProxy; class GPUProcessConnection : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr, public IPC::Connection::Client { public: - static RefPtr create(IPC::Connection& parentConnection); + static Ref create(Ref&&); ~GPUProcessConnection(); IPC::Connection& connection() { return m_connection.get(); } @@ -117,7 +117,7 @@ class GPUProcessConnection : public ThreadSafeRefCountedAndCanMakeThreadSafeWeak static constexpr Seconds defaultTimeout = 3_s; private: - GPUProcessConnection(IPC::Connection::Identifier&&); + GPUProcessConnection(Ref&&); bool waitForDidInitialize(); void invalidate(); diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp index 7de1fd39cf8f3..c01a14d8f9e9d 100644 --- a/Source/WebKit/WebProcess/WebProcess.cpp +++ b/Source/WebKit/WebProcess/WebProcess.cpp @@ -416,9 +416,11 @@ static void scheduleLogMemoryStatistics(LogMemoryStatisticsReason reason) }); } -void WebProcess::initializeWebProcess(WebProcessCreationParameters&& parameters) +void WebProcess::initializeWebProcess(WebProcessCreationParameters&& parameters, CompletionHandler&& completionHandler) { TraceScope traceScope(InitializeWebProcessStart, InitializeWebProcessEnd); + // Reply immediately so that the identity is available as soon as possible. + completionHandler(ProcessIdentity { ProcessIdentity::CurrentProcess }); ASSERT(m_pageMap.isEmpty()); @@ -1370,9 +1372,16 @@ GPUProcessConnection& WebProcess::ensureGPUProcessConnection() // If we've lost our connection to the GPU process (e.g. it crashed) try to re-establish it. if (!m_gpuProcessConnection) { - m_gpuProcessConnection = GPUProcessConnection::create(Ref { *parentProcessConnection() }); - if (!m_gpuProcessConnection) + auto connectionIdentifiers = IPC::Connection::createConnectionIdentifierPair(); + if (!connectionIdentifiers) CRASH(); + protectedParentProcessConnection()->send(Messages::WebProcessProxy::CreateGPUProcessConnection(WTFMove(connectionIdentifiers->client)), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply); + Ref gpuConnection = IPC::Connection::createServerConnection(WTFMove(connectionIdentifiers->server)); +#if ENABLE(IPC_TESTING_API) + if (gpuConnection->ignoreInvalidMessageForTesting()) + gpuConnection->setIgnoreInvalidMessageForTesting(); +#endif + m_gpuProcessConnection = GPUProcessConnection::create(WTFMove(gpuConnection)); for (auto& page : m_pageMap.values()) { // If page is null, then it is currently being constructed. if (page) diff --git a/Source/WebKit/WebProcess/WebProcess.h b/Source/WebKit/WebProcess/WebProcess.h index bbb52b866b029..73d5012676950 100644 --- a/Source/WebKit/WebProcess/WebProcess.h +++ b/Source/WebKit/WebProcess/WebProcess.h @@ -452,7 +452,7 @@ class WebProcess : public AuxiliaryProcess WebProcess(); ~WebProcess(); - void initializeWebProcess(WebProcessCreationParameters&&); + void initializeWebProcess(WebProcessCreationParameters&&, CompletionHandler&&); void platformInitializeWebProcess(WebProcessCreationParameters&); void setWebsiteDataStoreParameters(WebProcessDataStoreParameters&&); void platformSetWebsiteDataStoreParameters(WebProcessDataStoreParameters&&); diff --git a/Source/WebKit/WebProcess/WebProcess.messages.in b/Source/WebKit/WebProcess/WebProcess.messages.in index 90917050394dd..4c0ffe97f21a1 100644 --- a/Source/WebKit/WebProcess/WebProcess.messages.in +++ b/Source/WebKit/WebProcess/WebProcess.messages.in @@ -21,7 +21,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. messages -> WebProcess LegacyReceiver NotRefCounted { - InitializeWebProcess(struct WebKit::WebProcessCreationParameters processCreationParameters) + InitializeWebProcess(struct WebKit::WebProcessCreationParameters processCreationParameters) -> (WebCore::ProcessIdentity processIdentity) SetWebsiteDataStoreParameters(struct WebKit::WebProcessDataStoreParameters parameters) CreateWebPage(WebCore::PageIdentifier newPageID, struct WebKit::WebPageCreationParameters pageCreationParameters)