Simplify ZygoteForkDelegate API further

components/nacl/loader/nacl_helper_linux.cc

Index: components/nacl/loader/nacl_helper_linux.cc
diff --git a/components/nacl/loader/nacl_helper_linux.cc b/components/nacl/loader/nacl_helper_linux.cc
index e1a6a66abaddadf5df5dbfa8178fcf54347b6e1c..6288655ffeeefad6a2ad284dc3d15543a855b90d 100644
--- a/components/nacl/loader/nacl_helper_linux.cc
+++ b/components/nacl/loader/nacl_helper_linux.cc
@@ -27,6 +27,7 @@
 #include "base/posix/global_descriptors.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #include "base/process/kill.h"
+#include "base/process/process_handle.h"
 #include "base/rand_util.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "components/nacl/loader/nacl_listener.h"
@@ -111,27 +112,29 @@ void BecomeNaClLoader(const std::vector<int>& child_fds,
 // Start the NaCl loader in a child created by the NaCl loader Zygote.
 void ChildNaClLoaderInit(const std::vector<int>& child_fds,
                          const NaClLoaderSystemInfo& system_info,
-                         bool uses_nonsfi_mode) {
+                         bool uses_nonsfi_mode,
+                         const std::string& channel_id) {
   const int parent_fd = child_fds[content::ZygoteForkDelegate::kParentFDIndex];
   const int dummy_fd = child_fds[content::ZygoteForkDelegate::kDummyFDIndex];
+
   bool validack = false;
-  const size_t kMaxReadSize = 1024;
-  char buffer[kMaxReadSize];
+  base::ProcessId real_pid;
   // Wait until the parent process has discovered our PID.  We
   // should not fork any child processes (which the seccomp
   // sandbox does) until then, because that can interfere with the
   // parent's discovery of our PID.
-  const ssize_t nread = HANDLE_EINTR(read(parent_fd, buffer, kMaxReadSize));
-
-  if (nread < 0) {
-    perror("read");
-    LOG(ERROR) << "read returned " << nread;
-  } else if (nread > 0) {
-    VLOG(1) << "NaCl loader is synchronised with Chrome zygote";
+  const ssize_t nread =
+      HANDLE_EINTR(read(parent_fd, &real_pid, sizeof(real_pid)));

[Mark Seaborn, 2014/04/17 18:32:12]

So you're sending the real PID to the NaCl process even though it doesn't use
it?

It would be nice to hide the PID from Non-SFI NaCl processes, because the PID
leaks some information about system state.  We talked about this a bit here:
https://docs.google.com/a/chromium.org/document/d/1wL5SCBdbXFGpqNkmzxN8o2w2KP...

It would actually be better not to send the PID.

Alternatively, you could zero real_pid here after receiving it (with a comment).
 But this might need to be a volatile write to be effective at the end of a
function.

BTW, what does the real PID get used for in non-NaCl subprocesses?

[jln (very slow on Chromium), 2014/04/17 18:40:06]

We do use the real pid to reap processes no?

The NaCl helper, the Zygote and the browser reference pids through their
(browser-level) global PID.

Unfortunately, with 3 namespaces, it's the only common ground. We could (in
another change), make sure that this "common ground" has nothing to do with the
real pid on the system, but is it worth it?

[jln (very slow on Chromium), 2014/04/17 18:42:33]

(err, disregard, you're talking about ChildNaClLoaderInit, which runs in the
child).

+  if (static_cast<size_t>(nread) == sizeof(real_pid)) {
     CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-        switches::kProcessChannelID, std::string(buffer, nread));
+        switches::kProcessChannelID, channel_id);
     validack = true;
+  } else {
+    if (nread < 0)
+      perror("read");
+    LOG(ERROR) << "read returned " << nread;
   }
+
   if (IGNORE_EINTR(close(dummy_fd)) != 0)
     LOG(ERROR) << "close(dummy_fd) failed";
   if (IGNORE_EINTR(close(parent_fd)) != 0)
@@ -157,6 +160,12 @@ bool HandleForkRequest(const std::vector<int>& child_fds,
     return false;
   }
 
+  std::string channel_id;
+  if (!input_iter->ReadString(&channel_id)) {
+    LOG(ERROR) << "Could not read channel_id string";
+    return false;
+  }
+
   if (content::ZygoteForkDelegate::kNumPassedFDs != child_fds.size()) {
     LOG(ERROR) << "nacl_helper: unexpected number of fds, got "
         << child_fds.size();
@@ -170,7 +179,7 @@ bool HandleForkRequest(const std::vector<int>& child_fds,
   }
 
   if (child_pid == 0) {
-    ChildNaClLoaderInit(child_fds, system_info, uses_nonsfi_mode);
+    ChildNaClLoaderInit(child_fds, system_info, uses_nonsfi_mode, channel_id);
     NOTREACHED();
   }