Split NaCl SFI and non-SFI helpers into separate processes

components/nacl/zygote/nacl_fork_delegate_linux.cc

 // Copyright (c) 2012 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/zygote/nacl_fork_delegate_linux.h"
 
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/resource.h>
 #include <sys/socket.h>
 
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/cpu.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/path_service.h"
 #include "base/pickle.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/global_descriptors.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #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_paths.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "components/nacl/loader/nacl_helper_linux.h"
 #include "content/public/common/content_descriptors.h"
 #include "content/public/common/content_switches.h"
 #include "sandbox/linux/suid/client/setuid_sandbox_client.h"
 
 namespace {
 
 // Note these need to match up with their counterparts in nacl_helper_linux.c
 // and nacl_helper_bootstrap_linux.c.
 const char kNaClHelperReservedAtZero[] =
     "--reserved_at_zero=0xXXXXXXXXXXXXXXXX";
 const char kNaClHelperRDebug[] = "--r_debug=0xXXXXXXXXXXXXXXXX";
 
+// We only enable non-SFI mode by default on ChromeOS/ARMEL.
+#if defined(OS_CHROMEOS) || defined(ARCH_CPU_ARMEL)

[Mark Seaborn, 2014/05/10 00:10:40]

I think Julien wanted to de-duplicate this condition, so once Elijah's change is
committed, maybe you could rebase and move the definition of this constant to a
header file?

[jln (very slow on Chromium), 2014/05/10 00:44:50]

We could even make it a function that also queries the command line and return
whether or not to enable nonsfi. WDYT?

[mdempsky, 2014/05/10 01:58:52]

Looking at Elijah's pending CL, it doesn't look straight forward to me to
reconcile, but I can still give it a shot once his change has landed.

In the mean time, I restructured this code a bit to try to make it clearer.

[mdempsky, 2014/05/12 22:38:30]

Done: I've moved the IsNonSFIModeEnabled() function from nacl/renderer into
nacl/common, and I'm now also using it in nacl/zygote.

+const bool kEnableNonSFIByDefault = true;
+#else
+const bool kEnableNonSFIByDefault = false;
+#endif
+
 #if defined(ARCH_CPU_X86)
 bool NonZeroSegmentBaseIsSlow() {
   base::CPU cpuid;
   // Using a non-zero segment base is known to be very slow on Intel
   // Atom CPUs.  See "Segmentation-based Memory Protection Mechanism
   // on Intel Atom Microarchitecture: Coding Optimizations" (Leonardo
   // Potenza, Intel).
   //
   // The following list of CPU model numbers is taken from:
   // "Intel 64 and IA-32 Architectures Software Developer's Manual"
@@ skipping 46 matching lines @@
   if (msg_len <= 0) {
     LOG(ERROR) << "SendIPCRequestAndReadReply: RecvMsg failed";
     return false;
   }
   *reply_size = msg_len;
   return true;
 }
 
 }  // namespace.
 
-NaClForkDelegate::NaClForkDelegate()
-    : status_(kNaClHelperUnused),
-      fd_(-1) {}
+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];
+
+  if (nonsfi_mode_) {
+    // Only launch the non-SFI helper process if we support non-SFI by default
+    // for this build target, or it's explicitly enabled via the command line.
+    const bool enable_nonsfi = kEnableNonSFIByDefault ||
+                               CommandLine::ForCurrentProcess()->HasSwitch(
+                                   switches::kEnableNaClNonSfiMode);
+    if (!enable_nonsfi) {
+      return;
+    }
+  }
 
   scoped_ptr<sandbox::SetuidSandboxClient> setuid_sandbox_client(
       sandbox::SetuidSandboxClient::Create());
 
   // For communications between the NaCl loader process and
   // the SUID sandbox.
   int nacl_sandbox_descriptor =
       base::GlobalDescriptors::kBaseDescriptor + kSandboxIPCChannel;
   // 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));
 
   // Using nacl_helper_bootstrap is not necessary on x86-64 because
   // NaCl's x86-64 sandbox is not zero-address-based.  Starting
   // nacl_helper through nacl_helper_bootstrap works on x86-64, but it
   // leaves nacl_helper_bootstrap mapped at a fixed address at the
   // bottom of the address space, which is undesirable because it
   // effectively defeats ASLR.
@@ skipping 114 matching lines @@
   // TODO(bradchen): Make this LOG(ERROR) when the NaCl helper
   // becomes the default.
   fd_ = -1;
   if (IGNORE_EINTR(close(fds[0])) != 0)
     LOG(ERROR) << "close(fds[0]) failed";
 }
 
 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;
 }
 
 NaClForkDelegate::~NaClForkDelegate() {
   // side effect of close: delegate process will terminate
   if (status_ == kNaClHelperSuccess) {
     if (IGNORE_EINTR(close(fd_)) != 0)
       LOG(ERROR) << "close(fd_) failed";
   }
 }
 
 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;
 }
 
 pid_t NaClForkDelegate::Fork(const std::string& process_type,
                              const std::vector<int>& fds,
                              const std::string& channel_id) {
   VLOG(1) << "NaClForkDelegate::Fork";
 
   DCHECK(fds.size() == kNumPassedFDs);
 
   if (status_ != kNaClHelperSuccess) {
     LOG(ERROR) << "Cannot launch NaCl process: nacl_helper failed to start";
     return -1;
   }
 
   // First, send a remote fork request.
   Pickle write_pickle;
   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];
   ssize_t reply_size = 0;
   bool got_reply =
       SendIPCRequestAndReadReply(fd_, fds, write_pickle,
                                  reply_buf, sizeof(reply_buf), &reply_size);
   if (!got_reply) {
     LOG(ERROR) << "Could not perform remote fork.";
     return -1;
@@ skipping 47 matching lines @@
   int remote_exit_code;
   if (!iter.ReadInt(&remote_exit_code)) {
     LOG(ERROR) << "GetTerminationStatus: pickle failed";
     return false;
   }
 
   *status = static_cast<base::TerminationStatus>(termination_status);
   *exit_code = remote_exit_code;
   return true;
 }