Split NaCl SFI and non-SFI helpers into separate processes

With this change, NaCl SFI and non-SFI processes now run in disjoint
PID namespaces, so the kernel should prevent sending signals from an SFI
process to a non-SFI process, or vice versa.  (The NaCl PID namespaces
are still nested within the renderer's PID namespace though.)

BUG=364945
NOTRY=true

Review URL: https://codereview.chromium.org/279693002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270244 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 96 insertions, 29 deletions

diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc
index 486cd42..9d5f3f0 100644
--- a/chrome/app/chrome_main_delegate.cc
+++ b/chrome/app/chrome_main_delegate.cc
@@ -858,7 +858,7 @@ bool ChromeMainDelegate::DelaySandboxInitialization(
 void ChromeMainDelegate::ZygoteStarting(
     ScopedVector<content::ZygoteForkDelegate>* delegates) {
 #if !defined(DISABLE_NACL)
-  delegates->push_back(new NaClForkDelegate());
+  nacl::AddNaClZygoteForkDelegates(delegates);
 #endif
 }
 
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 821d4b1..4b554e7 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1703,6 +1703,7 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches(
     static const char* const kSwitchNames[] = {
       // Load (in-process) Pepper plugins in-process in the zygote pre-sandbox.
       switches::kDisableBundledPpapiFlash,
+      switches::kEnableNaClNonSfiMode,
       switches::kNaClDangerousNoSandboxNonSfi,
       switches::kPpapiFlashPath,
       switches::kPpapiFlashVersion,
diff --git a/components/nacl.gyp b/components/nacl.gyp
index a7b2538..d55e127 100644
--- a/components/nacl.gyp
+++ b/components/nacl.gyp
@@ -466,6 +466,8 @@
         'nacl/common/nacl_host_messages.cc',
         'nacl/common/nacl_messages.cc',
         'nacl/common/nacl_messages.h',
+        'nacl/common/nacl_nonsfi_util.cc',
+        'nacl/common/nacl_nonsfi_util.h',
         'nacl/common/nacl_process_type.h',
         'nacl/common/nacl_sandbox_type_mac.h',
         'nacl/common/nacl_types.cc',
diff --git a/components/nacl/common/nacl_nonsfi_util.cc b/components/nacl/common/nacl_nonsfi_util.cc
new file mode 100644
index 0000000..d56cea6
--- /dev/null
+++ b/components/nacl/common/nacl_nonsfi_util.cc
@@ -0,0 +1,24 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/nacl/common/nacl_nonsfi_util.h"
+
+#include "base/command_line.h"
+#include "build/build_config.h"
+#include "components/nacl/common/nacl_switches.h"
+
+namespace nacl {
+
+bool IsNonSFIModeEnabled() {
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
+  return true;
+#elif defined(OS_LINUX)
+  return CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableNaClNonSfiMode);
+#else
+  return false;
+#endif
+}
+
+}  // namespace nacl
diff --git a/components/nacl/common/nacl_nonsfi_util.h b/components/nacl/common/nacl_nonsfi_util.h
new file mode 100644
index 0000000..07c033f
--- /dev/null
+++ b/components/nacl/common/nacl_nonsfi_util.h
@@ -0,0 +1,18 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_NACL_COMMON_NACL_NONSFI_UTIL_H_
+#define COMPONENTS_NACL_COMMON_NACL_NONSFI_UTIL_H_
+
+namespace nacl {
+
+// Returns true if non-SFI mode *can* run on the current platform and if non-SFI
+// manifest entries are preferred.  There can be other restrictions which
+// prevent a particular module from launching.  See NaClProcessHost::Launch
+// which makes the final determination.
+bool IsNonSFIModeEnabled();
+
+}  // namespace nacl
+
+#endif  // COMPONENTS_NACL_COMMON_NACL_NONSFI_UTIL_H_
diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc
index 2aff86bd..429d99e 100644
--- a/components/nacl/renderer/ppb_nacl_private_impl.cc
+++ b/components/nacl/renderer/ppb_nacl_private_impl.cc
@@ -18,6 +18,7 @@
 #include "base/rand_util.h"
 #include "components/nacl/common/nacl_host_messages.h"
 #include "components/nacl/common/nacl_messages.h"
+#include "components/nacl/common/nacl_nonsfi_util.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "components/nacl/common/nacl_types.h"
 #include "components/nacl/renderer/histogram.h"
@@ -532,19 +533,8 @@ int32_t GetNumberOfProcessors() {
   return num_processors;
 }
 
-PP_Bool IsNonSFIModeEnabled() {
-// Note that this only indicates whether non-sfi mode *can* run for a given
-// platform and if nonsfi manifest entries are preferred.  There can be other
-// restrictions which prevent a particular module from launching.  See
-// NaClProcessHost::Launch which makes the final determination.
-#if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
-  return PP_TRUE;
-#elif defined(OS_LINUX)
-  return PP_FromBool(CommandLine::ForCurrentProcess()->HasSwitch(
-                         switches::kEnableNaClNonSfiMode));
-#else
-  return PP_FALSE;
-#endif
+PP_Bool PPIsNonSFIModeEnabled() {
+  return PP_FromBool(IsNonSFIModeEnabled());
 }
 
 int32_t GetNexeFd(PP_Instance instance,
@@ -1015,7 +1005,7 @@ int32_t CreateJsonManifest(PP_Instance instance,
       new nacl::JsonManifest(
           manifest_url,
           isa_type,
-          PP_ToBool(IsNonSFIModeEnabled()),
+          IsNonSFIModeEnabled(),
           PP_ToBool(NaClDebugEnabledForURL(manifest_url))));
   JsonManifest::ErrorInfo error_info;
   if (j->Init(manifest_data, &error_info)) {
@@ -1272,7 +1262,7 @@ const PPB_NaCl_Private nacl_interface = {
   &GetReadonlyPnaclFD,
   &CreateTemporaryFile,
   &GetNumberOfProcessors,
-  &IsNonSFIModeEnabled,
+  &PPIsNonSFIModeEnabled,
   &GetNexeFd,
   &ReportTranslationFinished,
   &OpenNaClExecutable,
diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.cc b/components/nacl/zygote/nacl_fork_delegate_linux.cc
index 0e5f8e7e..51a9aec 100644
--- a/components/nacl/zygote/nacl_fork_delegate_linux.cc
+++ b/components/nacl/zygote/nacl_fork_delegate_linux.cc
@@ -27,6 +27,8 @@
 #include "base/process/kill.h"
 #include "base/process/launch.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
+#include "build/build_config.h"
+#include "components/nacl/common/nacl_nonsfi_util.h"
 #include "components/nacl/common/nacl_paths.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "components/nacl/loader/nacl_helper_linux.h"
@@ -108,14 +110,26 @@ bool SendIPCRequestAndReadReply(int ipc_channel,
 
 }  // namespace.
 
-NaClForkDelegate::NaClForkDelegate()
-    : status_(kNaClHelperUnused),
-      fd_(-1) {}
+namespace nacl {
+
+void AddNaClZygoteForkDelegates(
+    ScopedVector<content::ZygoteForkDelegate>* delegates) {
+  delegates->push_back(new NaClForkDelegate(false /* nonsfi_mode */));
+  delegates->push_back(new NaClForkDelegate(true /* nonsfi_mode */));
+}
+
+NaClForkDelegate::NaClForkDelegate(bool nonsfi_mode)
+    : nonsfi_mode_(nonsfi_mode), status_(kNaClHelperUnused), fd_(-1) {
+}
 
 void NaClForkDelegate::Init(const int sandboxdesc,
                             const bool enable_layer1_sandbox) {
   VLOG(1) << "NaClForkDelegate::Init()";
-  int fds[2];
+
+  // Only launch the non-SFI helper process if non-SFI mode is enabled.
+  if (nonsfi_mode_ && !IsNonSFIModeEnabled()) {
+    return;
+  }
 
   scoped_ptr<sandbox::SetuidSandboxClient> setuid_sandbox_client(
       sandbox::SetuidSandboxClient::Create());
@@ -127,7 +141,8 @@ void NaClForkDelegate::Init(const int sandboxdesc,
   // Confirm a hard-wired assumption.
   DCHECK_EQ(sandboxdesc, nacl_sandbox_descriptor);
 
-  CHECK(socketpair(PF_UNIX, SOCK_SEQPACKET, 0, fds) == 0);
+  int fds[2];
+  PCHECK(0 == socketpair(PF_UNIX, SOCK_SEQPACKET, 0, fds));
   base::FileHandleMappingVector fds_to_map;
   fds_to_map.push_back(std::make_pair(fds[1], kNaClZygoteDescriptor));
   fds_to_map.push_back(std::make_pair(sandboxdesc, nacl_sandbox_descriptor));
@@ -262,7 +277,8 @@ void NaClForkDelegate::Init(const int sandboxdesc,
 void NaClForkDelegate::InitialUMA(std::string* uma_name,
                                   int* uma_sample,
                                   int* uma_boundary_value) {
-  *uma_name = "NaCl.Client.Helper.InitState";
+  *uma_name = nonsfi_mode_ ? "NaCl.Client.HelperNonSFI.InitState"
+                           : "NaCl.Client.Helper.InitState";
   *uma_sample = status_;
   *uma_boundary_value = kNaClHelperStatusBoundary;
 }
@@ -279,10 +295,14 @@ bool NaClForkDelegate::CanHelp(const std::string& process_type,
                                std::string* uma_name,
                                int* uma_sample,
                                int* uma_boundary_value) {
-  if (process_type != switches::kNaClLoaderProcess &&
-      process_type != switches::kNaClLoaderNonSfiProcess)
+  // We can only help with a specific process type depending on nonsfi_mode_.
+  const char* helpable_process_type = nonsfi_mode_
+                                          ? switches::kNaClLoaderNonSfiProcess
+                                          : switches::kNaClLoaderProcess;
+  if (process_type != helpable_process_type)
     return false;
-  *uma_name = "NaCl.Client.Helper.StateOnFork";
+  *uma_name = nonsfi_mode_ ? "NaCl.Client.HelperNonSFI.StateOnFork"
+                           : "NaCl.Client.Helper.StateOnFork";
   *uma_sample = status_;
   *uma_boundary_value = kNaClHelperStatusBoundary;
   return true;
@@ -305,9 +325,7 @@ pid_t NaClForkDelegate::Fork(const std::string& process_type,
   write_pickle.WriteInt(nacl::kNaClForkRequest);
   // TODO(hamaji): When we split the helper binary for non-SFI mode
   // from nacl_helper, stop sending this information.
-  const bool uses_nonsfi_mode =
-    process_type == switches::kNaClLoaderNonSfiProcess;
-  write_pickle.WriteBool(uses_nonsfi_mode);
+  write_pickle.WriteBool(nonsfi_mode_);
   write_pickle.WriteString(channel_id);
 
   char reply_buf[kNaClMaxIPCMessageLength];
@@ -375,3 +393,5 @@ bool NaClForkDelegate::GetTerminationStatus(pid_t pid, bool known_dead,
   *exit_code = remote_exit_code;
   return true;
 }
+
+}  // namespace nacl
diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.h b/components/nacl/zygote/nacl_fork_delegate_linux.h
index 4cc105c..10520fc 100644
--- a/components/nacl/zygote/nacl_fork_delegate_linux.h
+++ b/components/nacl/zygote/nacl_fork_delegate_linux.h
@@ -12,6 +12,15 @@
 #include "base/compiler_specific.h"
 #include "content/public/common/zygote_fork_delegate_linux.h"
 
+template <typename>
+class ScopedVector;
+
+namespace nacl {
+
+// Appends any ZygoteForkDelegate instances needed by NaCl to |*delegates|.
+void AddNaClZygoteForkDelegates(
+    ScopedVector<content::ZygoteForkDelegate>* delegates);
+
 // The NaClForkDelegate is created during Chrome linux zygote
 // initialization, and provides "fork()" functionality with
 // NaCl specific process characteristics (specifically address
@@ -19,7 +28,7 @@
 // A new delegate is passed in as an argument to ZygoteMain().
 class NaClForkDelegate : public content::ZygoteForkDelegate {
  public:
-  NaClForkDelegate();
+  NaClForkDelegate(bool nonsfi_mode);
   virtual ~NaClForkDelegate();
 
   virtual void Init(int sandboxdesc, bool enable_layer1_sandbox) OVERRIDE;
@@ -49,8 +58,11 @@ class NaClForkDelegate : public content::ZygoteForkDelegate {
     kNaClHelperStatusBoundary  // Must be one greater than highest value used.
   };
 
+  const bool nonsfi_mode_;
   NaClHelperStatus status_;
   int fd_;
 };
 
+}  // namespace nacl
+
 #endif  // COMPONENTS_NACL_ZYGOTE_NACL_FORK_DELEGATE_LINUX_H_