Bug 1278528: Don't try to initialize the sandbox TargetServices when we are not sandboxed. r=jimm
authorBob Owen <bobowencode@gmail.com>
Tue, 07 Jun 2016 14:03:51 +0100
changeset 300892 a29db603cf6ec486f6065b1e4471befda71fa6da
parent 300891 17c1f2315eb528745cfc7b73b1f5dc6cfcf7a35c
child 300893 dec55e893640ed288c34e3398b3c4037fb71511d
push id19599
push usercbook@mozilla.com
push dateWed, 08 Jun 2016 10:16:21 +0000
treeherderfx-team@81f4cc3f6f4c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1278528
milestone50.0a1
Bug 1278528: Don't try to initialize the sandbox TargetServices when we are not sandboxed. r=jimm MozReview-Commit-ID: EpXy9LYXwQL
browser/app/nsBrowserApp.cpp
ipc/contentproc/plugin-container.cpp
ipc/glue/GeckoChildProcessHost.cpp
security/sandbox/win/SandboxInitialization.h
--- a/browser/app/nsBrowserApp.cpp
+++ b/browser/app/nsBrowserApp.cpp
@@ -331,17 +331,17 @@ int main(int argc, char* argv[], char* e
 {
 #ifdef MOZ_BROWSER_CAN_BE_CONTENTPROC
   // We are launching as a content process, delegate to the appropriate
   // main
   if (argc > 1 && IsArg(argv[1], "contentproc")) {
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
     // We need to initialize the sandbox TargetServices before InitXPCOMGlue
     // because we might need the sandbox broker to give access to some files.
-    if (!sandboxing::GetInitializedTargetServices()) {
+    if (IsSandboxedProcess() && !sandboxing::GetInitializedTargetServices()) {
       Output("Failed to initialize the sandbox target services.");
       return 255;
     }
 #endif
 
     nsresult rv = InitXPCOMGlue(argv[0], nullptr);
     if (NS_FAILED(rv)) {
       return 255;
--- a/ipc/contentproc/plugin-container.cpp
+++ b/ipc/contentproc/plugin-container.cpp
@@ -72,22 +72,20 @@ InitializeBinder(void *aDummy) {
     MOZ_ASSERT(!err);
     LOGE_IF(err, "setpriority failed. Current process needs root permission.");
     android::ProcessState::self()->startThreadPool();
     setpriority(PRIO_PROCESS, 0, curPrio);
 }
 #endif
 
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
-static bool gIsSandboxEnabled = false;
-
 class WinSandboxStarter : public mozilla::gmp::SandboxStarter {
 public:
     virtual bool Start(const char *aLibPath) override {
-        if (gIsSandboxEnabled) {
+        if (IsSandboxedProcess()) {
             mozilla::sandboxing::LowerSandbox();
         }
         return true;
     }
 };
 #endif
 
 #if defined(XP_LINUX) && defined(MOZ_GMP_SANDBOX)
@@ -150,25 +148,22 @@ content_process_main(int argc, char* arg
     // forward here. We expect the last arg to be the child process type.
     if (argc < 1) {
       return 3;
     }
 
     bool isNuwa = false;
     for (int i = 1; i < argc; i++) {
         isNuwa |= strcmp(argv[i], "-nuwa") == 0;
-#if defined(XP_WIN) && defined(MOZ_SANDBOX)
-        gIsSandboxEnabled |= strcmp(argv[i], "-sandbox") == 0;
-#endif
     }
 
     XREChildData childData;
 
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
-    if (gIsSandboxEnabled) {
+    if (IsSandboxedProcess()) {
         childData.sandboxTargetServices =
             mozilla::sandboxing::GetInitializedTargetServices();
         if (!childData.sandboxTargetServices) {
             return 1;
         }
 
         childData.ProvideLogFunction = mozilla::sandboxing::ProvideLogFunction;
     }
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -1033,30 +1033,28 @@ GeckoChildProcessHost::PerformAsyncLaunc
     case GeckoProcessType_Content:
 #if defined(MOZ_CONTENT_SANDBOX)
       if (mSandboxLevel > 0 &&
           !PR_GetEnv("MOZ_DISABLE_CONTENT_SANDBOX")) {
         // For now we treat every failure as fatal in SetSecurityLevelForContentProcess
         // and just crash there right away. Should this change in the future then we
         // should also handle the error here.
         mSandboxBroker.SetSecurityLevelForContentProcess(mSandboxLevel);
-        cmdLine.AppendLooseValue(UTF8ToWide("-sandbox"));
         shouldSandboxCurrentProcess = true;
         AddContentSandboxAllowedFiles(mSandboxLevel, mAllowedFilesRead);
       }
 #endif // MOZ_CONTENT_SANDBOX
       break;
     case GeckoProcessType_Plugin:
       if (mSandboxLevel > 0 &&
           !PR_GetEnv("MOZ_DISABLE_NPAPI_SANDBOX")) {
         bool ok = mSandboxBroker.SetSecurityLevelForPluginProcess(mSandboxLevel);
         if (!ok) {
           return false;
         }
-        cmdLine.AppendLooseValue(UTF8ToWide("-sandbox"));
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_IPDLUnitTest:
       // XXX: We don't sandbox this process type yet
       break;
     case GeckoProcessType_GMPlugin:
       if (!PR_GetEnv("MOZ_DISABLE_GMP_SANDBOX")) {
@@ -1066,17 +1064,16 @@ GeckoChildProcessHost::PerformAsyncLaunc
         // so use sandbox level USER_RESTRICTED instead of USER_LOCKDOWN.
         bool isWidevine = std::any_of(aExtraOpts.begin(), aExtraOpts.end(),
           [](const std::string arg) { return arg.find("gmp-widevinecdm") != std::string::npos; });
         auto level = isWidevine ? SandboxBroker::Restricted : SandboxBroker::LockDown;
         bool ok = mSandboxBroker.SetSecurityLevelForGMPlugin(level);
         if (!ok) {
           return false;
         }
-        cmdLine.AppendLooseValue(UTF8ToWide("-sandbox"));
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_Default:
     default:
       MOZ_CRASH("Bad process type in GeckoChildProcessHost");
       break;
   };
--- a/security/sandbox/win/SandboxInitialization.h
+++ b/security/sandbox/win/SandboxInitialization.h
@@ -7,16 +7,20 @@
 #ifndef mozilla_sandboxing_SandboxInitialization_h
 #define mozilla_sandboxing_SandboxInitialization_h
 
 namespace sandbox {
 class BrokerServices;
 class TargetServices;
 }
 
+// Things that use this file will probably want access to the IsSandboxedProcess
+// function defined in one of the Chromium sandbox cc files.
+extern "C" bool IsSandboxedProcess();
+
 namespace mozilla {
 // Note the Chromium code just uses a bare sandbox namespace, which makes using
 // sandbox for our namespace painful.
 namespace sandboxing {
 
 /**
  * Initializes (if required) and returns the Chromium sandbox TargetServices.
  *