Bug 1256992: Initialize Windows sandbox BrokerServices before any child processes are created. r=aklotz, r=bholley, a=ritu
☠☠ backed out by 08693317e658 ☠ ☠
authorBob Owen <bobowencode@gmail.com>
Wed, 23 Mar 2016 08:10:43 +0000
changeset 325814 6bba1d23fd2ca665cff262aa60c752c6ea52dbbb
parent 325813 186ef42b33c1fe583a0ad83780b32404680fa217
child 325815 727c6549ed889477f1b2ecfbf51b1b81ad004dfd
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, bholley, ritu
bugs1256992
milestone47.0a2
Bug 1256992: Initialize Windows sandbox BrokerServices before any child processes are created. r=aklotz, r=bholley, a=ritu
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
@@ -204,18 +204,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")
 
@@ -3717,16 +3721,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;
@@ -4302,16 +4312,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");