Use RecvMsgWithPid to find zygote PID instead of chrome-sandbox

content/browser/zygote_host/zygote_host_impl_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 "content/browser/zygote_host/zygote_host_impl_linux.h"
 
+#include <string.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/scoped_file.h"
 #include "base/linux_util.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #include "base/process/launch.h"
 #include "base/process/memory.h"
+#include "base/process/process_handle.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "content/browser/renderer_host/render_sandbox_host_linux.h"
 #include "content/common/child_process_sandbox_support_impl_linux.h"
 #include "content/common/zygote_commands_linux.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
 #include "sandbox/linux/suid/client/setuid_sandbox_client.h"
 #include "sandbox/linux/suid/common/sandbox.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/switches.h"
 
 #if defined(USE_TCMALLOC)
 #include "third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h"
 #endif
 
 namespace content {
 
+// Returns true if proc is the same process as or a descendent process of

[jln (very slow on Chromium), 2014/04/26 00:54:10]

nit: |proc| is the prevailing style in new code.
nit2: you called it |root| not |ancestor|

[mdempsky, 2014/04/26 03:07:15]

Oops, done.

+// ancestor.
+static bool SameOrDescendantOf(base::ProcessId proc, base::ProcessId root) {
+  for (unsigned i = 0; i < 100; i++) {
+    if (proc == root)
+      return true;
+
+    // Walk up process tree.
+    base::ProcessHandle handle;
+    CHECK(base::OpenProcessHandle(proc, &handle));
+    proc = base::GetParentProcessId(handle);
+    base::CloseProcessHandle(handle);
+    if (proc < 0)
+      return false;
+  }
+
+  NOTREACHED();
+  return false;
+}
+
 // static
 ZygoteHost* ZygoteHost::GetInstance() {
   return ZygoteHostImpl::GetInstance();
 }
 
 ZygoteHostImpl::ZygoteHostImpl()
     : control_fd_(-1),
       control_lock_(),
       pid_(-1),
       init_(false),
@@ skipping 17 matching lines @@
   init_ = true;
 
   base::FilePath chrome_path;
   CHECK(PathService::Get(base::FILE_EXE, &chrome_path));
   CommandLine cmd_line(chrome_path);
 
   cmd_line.AppendSwitchASCII(switches::kProcessType, switches::kZygoteProcess);
 
   int fds[2];
   CHECK(socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) == 0);
+  CHECK(UnixDomainSocket::EnableReceiveProcessId(fds[0]));
   base::FileHandleMappingVector fds_to_map;
   fds_to_map.push_back(std::make_pair(fds[1], kZygoteSocketPairFd));
 
   base::LaunchOptions options;
   const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess();
   if (browser_command_line.HasSwitch(switches::kZygoteCmdPrefix)) {
     cmd_line.PrependWrapper(
         browser_command_line.GetSwitchValueNative(switches::kZygoteCmdPrefix));
   }
   // Append any switches from the browser process that need to be forwarded on
@@ skipping 23 matching lines @@
   sandbox_binary_ = sandbox_cmd.c_str();
 
   // A non empty sandbox_cmd means we want a SUID sandbox.
   using_suid_sandbox_ = !sandbox_cmd.empty();
 
   // Start up the sandbox host process and get the file descriptor for the
   // renderers to talk to it.
   const int sfd = RenderSandboxHostLinux::GetInstance()->GetRendererSocket();
   fds_to_map.push_back(std::make_pair(sfd, GetSandboxFD()));
 
-  int dummy_fd = -1;
+  base::ScopedFD dummy_fd;
   if (using_suid_sandbox_) {
     scoped_ptr<sandbox::SetuidSandboxClient>
         sandbox_client(sandbox::SetuidSandboxClient::Create());
     sandbox_client->PrependWrapper(&cmd_line, &options);
     sandbox_client->SetupLaunchEnvironment();
 
+    // We no longer need this dummy socket for discovering the zygote's PID,
+    // but the sandbox is still hard-coded to expect a file descriptor at
+    // kZygoteIdFd. Fixing this requires a sandbox API change. :(
     CHECK_EQ(kZygoteIdFd, sandbox_client->GetUniqueToChildFileDescriptor());
-    dummy_fd = socket(AF_UNIX, SOCK_DGRAM, 0);
-    CHECK(dummy_fd >= 0);
-    fds_to_map.push_back(std::make_pair(dummy_fd, kZygoteIdFd));
+    dummy_fd.reset(socket(AF_UNIX, SOCK_DGRAM, 0));
+    CHECK_GE(dummy_fd.get(), 0);
+    fds_to_map.push_back(std::make_pair(dummy_fd.get(), kZygoteIdFd));
   }
 
   base::ProcessHandle process = -1;
   options.fds_to_remap = &fds_to_map;
   base::LaunchProcess(cmd_line.argv(), options, &process);
   CHECK(process != -1) << "Failed to launch zygote process";
+  dummy_fd.reset();
 
   if (using_suid_sandbox_) {
     // In the SUID sandbox, the real zygote is forked from the sandbox.
-    // We need to look for it.
-    // But first, wait for the zygote to tell us it's running.
+    // Wait for the zygote to tell us it's running, and receive its PID.

[jln (very slow on Chromium), 2014/04/26 00:54:10]

Do you want to add a longer comment here?

Let's explain that there is a PID namespace and that the kernel will
automatically translate the PID sent via the UnixDomainSocket().

[mdempsky, 2014/04/26 03:07:15]

Done.  PTAL and let me know if you think the new wording is clear enough.

     // The sending code is in content/browser/zygote_main_linux.cc.
     std::vector<int> fds_vec;
-    const int kExpectedLength = sizeof(kZygoteHelloMessage);
+    const size_t kExpectedLength = sizeof(kZygoteHelloMessage);
     char buf[kExpectedLength];
-    const ssize_t len = UnixDomainSocket::RecvMsg(fds[0], buf, sizeof(buf),
-                                                  &fds_vec);
-    CHECK_EQ(kExpectedLength, len) << "Incorrect zygote magic length";
-    CHECK(0 == strcmp(buf, kZygoteHelloMessage)) << "Incorrect zygote hello";
+    const ssize_t len = UnixDomainSocket::RecvMsgWithPid(
+        fds[0], buf, sizeof(buf), &fds_vec, &pid_);
+    CHECK_EQ(kExpectedLength, static_cast<size_t>(len))
+        << "Incorrect zygote magic length";
+    CHECK_EQ(0, memcmp(buf, kZygoteHelloMessage, kExpectedLength))
+        << "Incorrect zygote hello";
+    CHECK_EQ(0U, fds_vec.size())
+        << "Zygote hello should not include file descriptors";
 
-    std::string inode_output;
-    ino_t inode = 0;
-    // Figure out the inode for |dummy_fd|, close |dummy_fd| on our end,
-    // and find the zygote process holding |dummy_fd|.
-    CHECK(base::FileDescriptorGetInode(&inode, dummy_fd))
-        << "Cannot get inode for dummy_fd " << dummy_fd;
-    close(dummy_fd);
-
-    std::vector<std::string> get_inode_cmdline;
-    get_inode_cmdline.push_back(sandbox_binary_);
-    get_inode_cmdline.push_back(base::kFindInodeSwitch);
-    get_inode_cmdline.push_back(base::Int64ToString(inode));
-    CommandLine get_inode_cmd(get_inode_cmdline);
-    CHECK(base::GetAppOutput(get_inode_cmd, &inode_output))
-        << "Find inode command failed for inode " << inode;
-
-    base::TrimWhitespaceASCII(inode_output, base::TRIM_ALL, &inode_output);
-    CHECK(base::StringToInt(inode_output, &pid_))
-        << "Invalid find inode output: " << inode_output;
-    CHECK(pid_ > 0) << "Did not find zygote process (using sandbox binary "
-        << sandbox_binary_ << ")";
+    static const char kInvalidPID[] =
+        "Received invalid process ID for zygote; kernel might be too old? "
+        "See crbug.com/357670.";

[jln (very slow on Chromium), 2014/04/26 00:54:10]

Let's add a note that they can use "--" + kDisableSetuidSandbox to work around
this?

[mdempsky, 2014/04/26 03:07:15]

Done.

+    CHECK_GE(pid_, 1) << kInvalidPID;
+    CHECK(SameOrDescendantOf(pid_, base::GetProcId(process))) << kInvalidPID;
 
     if (process != pid_) {
       // Reap the sandbox.
       base::EnsureProcessGetsReaped(process);
     }
   } else {
     // Not using the SUID sandbox.
     pid_ = process;
   }
 
@@ skipping 315 matching lines @@
   return RenderSandboxHostLinux::GetInstance()->pid();
 }
 
 int ZygoteHostImpl::GetSandboxStatus() const {
   if (have_read_sandbox_status_word_)
     return sandbox_status_;
   return 0;
 }
 
 }  // namespace content