Bug 1256992: Initialize Windows sandbox BrokerServices before any child processes are created. r=aklotz, r=bholley
authorBob Owen <bobowencode@gmail.com>
Wed, 23 Mar 2016 08:10:43 +0000
changeset 329866 f2582471724f12b2eadde1288474b7574605dba1
parent 329865 8786bbb9702f808a01ae45a5153440a7c44699fb
child 329867 8573ffb50d4754f2eceece2347cee4053bdc8799
push id1146
push userCallek@gmail.com
push dateMon, 25 Jul 2016 16:35:44 +0000
treeherdermozilla-release@a55778f9cd5a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, bholley
bugs1256992
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1256992: Initialize Windows sandbox BrokerServices before any child processes are created. r=aklotz, r=bholley
js/xpconnect/src/XPCShellImpl.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.h
toolkit/xre/nsAppRunner.cpp
--- a/js/xpconnect/src/XPCShellImpl.cpp
+++ b/js/xpconnect/src/XPCShellImpl.cpp
@@ -39,16 +39,19 @@
 
 #ifdef ANDROID
 #include <android/log.h>
 #endif
 
 #ifdef XP_WIN
 #include "mozilla/widget/AudioSession.h"
 #include <windows.h>
+#if defined(MOZ_SANDBOX)
+#include "SandboxBroker.h"
+#endif
 #endif
 
 // all this crap is needed to do the interactive shell stuff
 #include <stdlib.h>
 #include <errno.h>
 #ifdef HAVE_IO_H
 #include <io.h>     /* for isatty() */
 #endif
@@ -1513,16 +1516,24 @@ XRE_XPCShellMain(int argc, char** argv, 
         // Initialize graphics prefs on the main thread, if not already done
         gfxPrefs::GetSingleton();
         // Initialize e10s check on the main thread, if not already done
         BrowserTabsRemoteAutostart();
 #ifdef XP_WIN
         // 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
 
         {
             JS::Rooted<JSObject*> glob(cx, holder->GetJSObject());
             if (!glob) {
                 return 1;
             }
 
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -12,31 +12,40 @@
 #include "sandbox/win/src/security_level.h"
 #include "mozilla/sandboxing/sandboxLogging.h"
 
 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()
 {
-  // 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;
-      }
-    }
+  if (sBrokerService) {
+    mPolicy = sBrokerService->CreatePolicy();
+  } else {
+    mPolicy = nullptr;
   }
-
-  mPolicy = sBrokerService->CreatePolicy();
 }
 
 bool
 SandboxBroker::LaunchApp(const wchar_t *aPath,
                          const wchar_t *aArguments,
                          const bool aEnableLogging,
                          void **aProcessHandle)
 {
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
@@ -22,16 +22,19 @@ namespace sandbox {
 }
 
 namespace mozilla {
 
 class SANDBOX_EXPORT SandboxBroker
 {
 public:
   SandboxBroker();
+
+  static bool Initialize();
+
   bool LaunchApp(const wchar_t *aPath,
                  const wchar_t *aArguments,
                  const bool aEnableLogging,
                  void **aProcessHandle);
   virtual ~SandboxBroker();
 
   // Security levels for different types of processes
 #if defined(MOZ_CONTENT_SANDBOX)
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -205,18 +205,22 @@
 #ifdef MOZ_B2G_LOADER
 #include "ProcessUtils.h"
 #endif
 
 #ifdef MOZ_WIDGET_ANDROID
 #include "AndroidBridge.h"
 #endif
 
-#if defined(MOZ_SANDBOX) && defined(XP_LINUX) && !defined(ANDROID)
+#if defined(MOZ_SANDBOX)
+#if defined(XP_LINUX) && !defined(ANDROID)
 #include "mozilla/SandboxInfo.h"
+#elif defined(XP_WIN)
+#include "SandboxBroker.h"
+#endif
 #endif
 
 extern uint32_t gRestartMode;
 extern void InstallSignalHandlers(const char *ProgramName);
 
 #define FILE_COMPATIBILITY_INFO NS_LITERAL_CSTRING("compatibility.ini")
 #define FILE_INVALIDATE_CACHES NS_LITERAL_CSTRING(".purgecaches")
 
@@ -3720,16 +3724,22 @@ XREMain::XRE_mainStartup(bool* aExitFlag
   if (!gtk_parse_args(&gArgc, &gArgv))
     return 1;
 #endif /* MOZ_WIDGET_GTK */
 
   if (PR_GetEnv("MOZ_RUN_GTEST")) {
     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) {
       gIsGtest = true;
       result = mozilla::RunGTest();
       gIsGtest = false;
     } else {
       result = 1;
@@ -4305,16 +4315,30 @@ XREMain::XRE_mainRun()
 
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP
   if (PR_GetEnv("MOZ_INSTRUMENT_EVENT_LOOP")) {
     bool logToConsole = true;
     mozilla::InitEventTracing(logToConsole);
   }
 #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
 
   {
     rv = appStartup->Run();
     if (NS_FAILED(rv)) {
       NS_ERROR("failed to run appstartup");