Tell nacl_helper to use non SFI mode in HandleForkRequest

components/nacl/zygote/nacl_fork_delegate_linux.cc

Index: components/nacl/zygote/nacl_fork_delegate_linux.cc
diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.cc b/components/nacl/zygote/nacl_fork_delegate_linux.cc
index 1b9c7af7765f0a855966baba96b32765784c6f3f..68f7f1affadf93f6a65d2b5597353c47ad843462 100644
--- a/components/nacl/zygote/nacl_fork_delegate_linux.cc
+++ b/components/nacl/zygote/nacl_fork_delegate_linux.cc
@@ -106,7 +106,8 @@ bool SendIPCRequestAndReadReply(int ipc_channel,
 
 NaClForkDelegate::NaClForkDelegate()
     : status_(kNaClHelperUnused),
-      fd_(-1) {}
+      fd_(-1),
+      uses_nonsfi_mode_(false) {}
 
 void NaClForkDelegate::Init(const int sandboxdesc) {
   VLOG(1) << "NaClForkDelegate::Init()";
@@ -252,8 +253,17 @@ bool NaClForkDelegate::CanHelp(const std::string& process_type,
                                std::string* uma_name,
                                int* uma_sample,
                                int* uma_boundary_value) {
-  if (process_type != switches::kNaClLoaderProcess)
+  if (process_type != switches::kNaClLoaderProcess &&
+      process_type != switches::kNaClNonSfiLoaderProcess)
     return false;
+  // We decide whether we will use SFI mode or non-SFI for the next
+  // run based on the process type here.
+  // TODO(hamaji): Have two helpers in content::Zygote and each
+  // NaClForkDelegate should focus on a single mode. This must be done
+  // when we split the helper binary for non-SFI mode from
+  // nacl_helper. Once this has been done, we can remove this check
+  // and uses_nonsfi_mode_ field.
+  uses_nonsfi_mode_ = process_type == switches::kNaClNonSfiLoaderProcess;

[Mark Seaborn, 2014/03/28 15:38:42]

Having CanHelp() store state that's later used by Fork() doesn't seem like a
good idea.  That creates a very subtle gotcha.  The interface in
content/public/common/zygote_fork_delegate_linux.h doesn't say that CanHelp()
must be called immediately after Fork() for the same subprocess request.

Maybe add a process_type argument to Fork()?

It's probably better not to add a use_nonsfi_mode argument to Fork(), because
content's ZygoteForkDelegate interface is maintaining a pretence that it doesn't
depend on NaCl (which isn't entirely true :-) ).

+jln for this aspect.

[hamaji, 2014/03/30 03:31:35]

Thanks for the suggestion! Yeah, as I've written in the seccomp patch, I was not
sure if this is a good idea. I was not also sure if changing content/ makes
sense for NaCl (though it seemed ZygoteForkDelegate is only for NaCl).

Done, by adding process_type argument.

   *uma_name = "NaCl.Client.Helper.StateOnFork";
   *uma_sample = status_;
   *uma_boundary_value = kNaClHelperStatusBoundary;
@@ -273,6 +283,10 @@ pid_t NaClForkDelegate::Fork(const std::vector<int>& fds) {
   // First, send a remote fork request.
   Pickle write_pickle;
   write_pickle.WriteInt(nacl::kNaClForkRequest);
+  // Tell nacl_helper whether it should use SFI mode or non-SFI mode.
+  // TODO(hamaji): Remove this once we have splitted nacl_helper into
+  // two helper binaries. See the comment in CanHelp as well.
+  write_pickle.WriteBool(uses_nonsfi_mode_);
 
   char reply_buf[kNaClMaxIPCMessageLength];
   ssize_t reply_size = 0;