Title: [250799] trunk/Source/WebKit
- Revision
- 250799
- Author
- pvol...@apple.com
- Date
- 2019-10-07 15:14:41 -0700 (Mon, 07 Oct 2019)
Log Message
[macOS] Layering violation in AuxiliaryProcessProxy::didFinishLaunching
https://bugs.webkit.org/show_bug.cgi?id=201617
Reviewed by Brent Fulgham.
The commit <https://trac.webkit.org/changeset/249649> introduced a layering violation in AuxiliaryProcessProxy::didFinishLaunching
where we inspect the pending message queue looking for a local file load message which needs the PID to create a sandbox extension
for the WebContent process. The layering violation can be fixed by creating a virtual method in AuxiliaryProcessProxy and override
the method in the WebProcessProxy to do the work needed to replace the message with a load request message containing a sandbox
extension created using the PID of the WebContent process. No new tests have been created, since this is covered by existing tests.
* UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::didFinishLaunching):
* UIProcess/AuxiliaryProcessProxy.h:
(WebKit::AuxiliaryProcessProxy::shouldSendPendingMessage):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::shouldSendPendingMessage):
* UIProcess/WebProcessProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (250798 => 250799)
--- trunk/Source/WebKit/ChangeLog 2019-10-07 22:14:27 UTC (rev 250798)
+++ trunk/Source/WebKit/ChangeLog 2019-10-07 22:14:41 UTC (rev 250799)
@@ -1,3 +1,24 @@
+2019-10-07 Per Arne Vollan <pvol...@apple.com>
+
+ [macOS] Layering violation in AuxiliaryProcessProxy::didFinishLaunching
+ https://bugs.webkit.org/show_bug.cgi?id=201617
+
+ Reviewed by Brent Fulgham.
+
+ The commit <https://trac.webkit.org/changeset/249649> introduced a layering violation in AuxiliaryProcessProxy::didFinishLaunching
+ where we inspect the pending message queue looking for a local file load message which needs the PID to create a sandbox extension
+ for the WebContent process. The layering violation can be fixed by creating a virtual method in AuxiliaryProcessProxy and override
+ the method in the WebProcessProxy to do the work needed to replace the message with a load request message containing a sandbox
+ extension created using the PID of the WebContent process. No new tests have been created, since this is covered by existing tests.
+
+ * UIProcess/AuxiliaryProcessProxy.cpp:
+ (WebKit::AuxiliaryProcessProxy::didFinishLaunching):
+ * UIProcess/AuxiliaryProcessProxy.h:
+ (WebKit::AuxiliaryProcessProxy::shouldSendPendingMessage):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::shouldSendPendingMessage):
+ * UIProcess/WebProcessProxy.h:
+
2019-10-07 Dean Jackson <d...@apple.com>
Provide options for DTTZ to happen in more situations
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp (250798 => 250799)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-10-07 22:14:27 UTC (rev 250798)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-10-07 22:14:41 UTC (rev 250799)
@@ -27,9 +27,7 @@
#include "AuxiliaryProcessProxy.h"
#include "AuxiliaryProcessMessages.h"
-#include "LoadParameters.h"
#include "Logging.h"
-#include "WebPageMessages.h"
#include "WebPageProxy.h"
#include "WebProcessProxy.h"
#include <wtf/RunLoop.h>
@@ -211,26 +209,10 @@
m_connection->open();
for (auto&& pendingMessage : std::exchange(m_pendingMessages, { })) {
+ if (!shouldSendPendingMessage(pendingMessage))
+ continue;
auto encoder = WTFMove(pendingMessage.encoder);
auto sendOptions = pendingMessage.sendOptions;
-#if HAVE(SANDBOX_ISSUE_MACH_EXTENSION_TO_PROCESS_BY_PID)
- if (encoder->messageName() == "LoadRequestWaitingForPID") {
- auto buffer = encoder->buffer();
- auto bufferSize = encoder->bufferSize();
- std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { });
- LoadParameters loadParameters;
- URL resourceDirectoryURL;
- WebPageProxyIdentifier pageID;
- if (decoder->decode(loadParameters) && decoder->decode(resourceDirectoryURL) && decoder->decode(pageID)) {
- if (auto* page = WebProcessProxy::webPage(pageID)) {
- page->maybeInitializeSandboxExtensionHandle(static_cast<WebProcessProxy&>(*this), loadParameters.request.url(), resourceDirectoryURL, loadParameters.sandboxExtensionHandle);
- send(Messages::WebPage::LoadRequest(loadParameters), decoder->destinationID());
- }
- } else
- ASSERT_NOT_REACHED();
- continue;
- }
-#endif
if (pendingMessage.asyncReplyInfo)
IPC::addAsyncReplyHandler(*connection(), pendingMessage.asyncReplyInfo->second, WTFMove(pendingMessage.asyncReplyInfo->first));
m_connection->sendMessage(WTFMove(encoder), sendOptions);
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h (250798 => 250799)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h 2019-10-07 22:14:27 UTC (rev 250798)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h 2019-10-07 22:14:41 UTC (rev 250799)
@@ -122,16 +122,18 @@
virtual void getLaunchOptions(ProcessLauncher::LaunchOptions&);
virtual void platformGetLaunchOptions(ProcessLauncher::LaunchOptions&) { };
-private:
- virtual void connectionWillOpen(IPC::Connection&);
- virtual void processWillShutDown(IPC::Connection&) = 0;
-
struct PendingMessage {
std::unique_ptr<IPC::Encoder> encoder;
OptionSet<IPC::SendOption> sendOptions;
Optional<std::pair<CompletionHandler<void(IPC::Decoder*)>, uint64_t>> asyncReplyInfo;
};
-
+
+ virtual bool shouldSendPendingMessage(const PendingMessage&) { return true; }
+
+private:
+ virtual void connectionWillOpen(IPC::Connection&);
+ virtual void processWillShutDown(IPC::Connection&) = 0;
+
Vector<PendingMessage> m_pendingMessages;
RefPtr<ProcessLauncher> m_processLauncher;
RefPtr<IPC::Connection> m_connection;
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (250798 => 250799)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-10-07 22:14:27 UTC (rev 250798)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-10-07 22:14:41 UTC (rev 250799)
@@ -31,6 +31,7 @@
#include "APIPageHandle.h"
#include "DataReference.h"
#include "DownloadProxyMap.h"
+#include "LoadParameters.h"
#include "Logging.h"
#include "PluginInfoStore.h"
#include "PluginProcessManager.h"
@@ -43,6 +44,7 @@
#include "WebNavigationDataStore.h"
#include "WebNotificationManagerProxy.h"
#include "WebPageGroup.h"
+#include "WebPageMessages.h"
#include "WebPageProxy.h"
#include "WebPasteboardProxy.h"
#include "WebProcessCache.h"
@@ -302,6 +304,29 @@
}
#endif
+bool WebProcessProxy::shouldSendPendingMessage(const PendingMessage& message)
+{
+#if HAVE(SANDBOX_ISSUE_MACH_EXTENSION_TO_PROCESS_BY_PID)
+ if (message.encoder->messageName() == "LoadRequestWaitingForPID") {
+ auto buffer = message.encoder->buffer();
+ auto bufferSize = message.encoder->bufferSize();
+ std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { });
+ LoadParameters loadParameters;
+ URL resourceDirectoryURL;
+ WebPageProxyIdentifier pageID;
+ if (decoder->decode(loadParameters) && decoder->decode(resourceDirectoryURL) && decoder->decode(pageID)) {
+ if (auto* page = WebProcessProxy::webPage(pageID)) {
+ page->maybeInitializeSandboxExtensionHandle(static_cast<WebProcessProxy&>(*this), loadParameters.request.url(), resourceDirectoryURL, loadParameters.sandboxExtensionHandle);
+ send(Messages::WebPage::LoadRequest(loadParameters), decoder->destinationID());
+ }
+ } else
+ ASSERT_NOT_REACHED();
+ return false;
+ }
+#endif
+ return true;
+}
+
void WebProcessProxy::connectionWillOpen(IPC::Connection& connection)
{
ASSERT(this->connection() == &connection);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (250798 => 250799)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-10-07 22:14:27 UTC (rev 250798)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-10-07 22:14:41 UTC (rev 250799)
@@ -329,7 +329,8 @@
void platformGetLaunchOptions(ProcessLauncher::LaunchOptions&) override;
void connectionWillOpen(IPC::Connection&) override;
void processWillShutDown(IPC::Connection&) override;
-
+ bool shouldSendPendingMessage(const PendingMessage&) final;
+
// ProcessLauncher::Client
void didFinishLaunching(ProcessLauncher*, IPC::Connection::Identifier) override;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
