From 36d7bd2f09736c517a113f8e29267cda9864d7b6 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Mon, 28 Mar 2016 10:51:05 -0700 Subject: [PATCH] Backed out changeset 30de9ac21a78 (bug 1256992) for causing crashes a=backout MozReview-Commit-ID: L30XEzrSo0y --- js/xpconnect/src/XPCShellImpl.cpp | 11 ------- .../win/src/sandboxbroker/sandboxBroker.cpp | 33 +++++++------------ .../win/src/sandboxbroker/sandboxBroker.h | 3 -- toolkit/xre/nsAppRunner.cpp | 26 +-------------- 4 files changed, 13 insertions(+), 60 deletions(-) diff --git a/js/xpconnect/src/XPCShellImpl.cpp b/js/xpconnect/src/XPCShellImpl.cpp index 3cb1e8b3d12..487c560bf90 100644 --- a/js/xpconnect/src/XPCShellImpl.cpp +++ b/js/xpconnect/src/XPCShellImpl.cpp @@ -44,9 +44,6 @@ #ifdef XP_WIN #include "mozilla/widget/AudioSession.h" #include -#if defined(MOZ_SANDBOX) -#include "SandboxBroker.h" -#endif #endif // all this crap is needed to do the interactive shell stuff @@ -1521,14 +1518,6 @@ XRE_XPCShellMain(int argc, char** argv, char** envp) // Plugin may require audio session if installed plugin can initialize // asynchronized. AutoAudioSession audioSession; - -#if defined(MOZ_SANDBOX) - // Required for sandboxed child processes. - if (!SandboxBroker::Initialize()) { - NS_WARNING("Failed to initialize broker services, sandboxed " - "processes will fail to start."); - } -#endif #endif { diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp index 8744055fac9..d584f9560ff 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp @@ -17,30 +17,21 @@ namespace mozilla sandbox::BrokerServices *SandboxBroker::sBrokerService = nullptr; -/* static */ -bool -SandboxBroker::Initialize() -{ - sBrokerService = sandbox::SandboxFactory::GetBrokerServices(); - if (!sBrokerService) { - return false; - } - - if (sBrokerService->Init() != sandbox::SBOX_ALL_OK) { - sBrokerService = nullptr; - return false; - } - - return true; -} - SandboxBroker::SandboxBroker() { - if (sBrokerService) { - mPolicy = sBrokerService->CreatePolicy(); - } else { - mPolicy = nullptr; + // XXX: This is not thread-safe! Two threads could simultaneously try + // to set `sBrokerService` + if (!sBrokerService) { + sBrokerService = sandbox::SandboxFactory::GetBrokerServices(); + if (sBrokerService) { + sandbox::ResultCode result = sBrokerService->Init(); + if (result != sandbox::SBOX_ALL_OK) { + sBrokerService = nullptr; + } + } } + + mPolicy = sBrokerService->CreatePolicy(); } bool diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h index c3f29f17eed..3a54c783647 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h @@ -27,9 +27,6 @@ class SANDBOX_EXPORT SandboxBroker { public: SandboxBroker(); - - static bool Initialize(); - bool LaunchApp(const wchar_t *aPath, const wchar_t *aArguments, const bool aEnableLogging, diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 3a51cb999ba..14da570533a 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -209,12 +209,8 @@ #include "AndroidBridge.h" #endif -#if defined(MOZ_SANDBOX) -#if defined(XP_LINUX) && !defined(ANDROID) +#if defined(MOZ_SANDBOX) && defined(XP_LINUX) && !defined(ANDROID) #include "mozilla/SandboxInfo.h" -#elif defined(XP_WIN) -#include "SandboxBroker.h" -#endif #endif extern uint32_t gRestartMode; @@ -3755,12 +3751,6 @@ XREMain::XRE_mainStartup(bool* aExitFlag) int result; #ifdef XP_WIN UseParentConsole(); -#if defined(MOZ_SANDBOX) - if (!SandboxBroker::Initialize()) { - NS_WARNING("Failed to initialize broker services, sandboxed processes " - "will fail to start."); - } -#endif #endif // RunGTest will only be set if we're in xul-unit if (mozilla::RunGTest) { @@ -4346,20 +4336,6 @@ XREMain::XRE_mainRun() } #endif /* MOZ_INSTRUMENT_EVENT_LOOP */ -#if defined(MOZ_SANDBOX) && defined(XP_WIN) - if (!SandboxBroker::Initialize()) { -#if defined(MOZ_CONTENT_SANDBOX) - // If we're sandboxing content and we fail to initialize, then crashing here - // seems like the sensible option. - if (BrowserTabsRemoteAutostart()) { - MOZ_CRASH("Failed to initialize broker services, can't continue."); - } -#endif - // Otherwise just warn for the moment, as most things will work. - NS_WARNING("Failed to initialize broker services, sandboxed processes will " - "fail to start."); - } -#endif #if (defined(XP_WIN) || defined(XP_MACOSX)) && defined(MOZ_CONTENT_SANDBOX) SetUpSandboxEnvironment(); #endif