Use RecvMsgWithPid to find zygote PID instead of chrome-sandbox

BUG=357670
NOTRY=true

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

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

3 files changed, 62 insertions, 48 deletions

diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc
index ad416f6..94bdc16 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.cc
+++ b/content/browser/zygote_host/zygote_host_impl_linux.cc
@@ -4,6 +4,7 @@
 
 #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>
@@ -25,6 +26,7 @@
 #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"
@@ -46,6 +48,26 @@
 
 namespace content {
 
+// Returns true if |proc| is the same process as or a descendent process of
+// |ancestor|.
+static bool SameOrDescendantOf(base::ProcessId proc, base::ProcessId ancestor) {
+  for (unsigned i = 0; i < 100; i++) {
+    if (proc == ancestor)
+      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();
@@ -83,6 +105,7 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
 
   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));
 
@@ -125,58 +148,52 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
   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.
+    // In the SUID sandbox, the real zygote is forked from the sandbox
+    // and will be executing in another PID namespace.
+    // Wait for the zygote to tell us it's running, and receive its PID,
+    // which the kernel will translate to our PID namespace.
     // 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";
-
-    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_ << ")";
+    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";
+
+    if (pid_ <= 0 || !SameOrDescendantOf(pid_, base::GetProcId(process))) {
+      LOG(FATAL)
+          << "Received invalid process ID for zygote; kernel might be too old? "
+             "See crbug.com/357670 or try using --"
+          << switches::kDisableSetuidSandbox << " to workaround.";
+    }
 
     if (process != pid_) {
       // Reap the sandbox.
diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc
index 63c472f..d2dc66a 100644
--- a/content/zygote/zygote_linux.cc
+++ b/content/zygote/zygote_linux.cc
@@ -83,10 +83,10 @@ bool Zygote::ProcessRequests() {
   if (UsingSUIDSandbox()) {
     // Let the ZygoteHost know we are ready to go.
     // The receiving code is in content/browser/zygote_host_linux.cc.
-    std::vector<int> empty;
     bool r = UnixDomainSocket::SendMsg(kZygoteSocketPairFd,
                                        kZygoteHelloMessage,
-                                       sizeof(kZygoteHelloMessage), empty);
+                                       sizeof(kZygoteHelloMessage),
+                                       std::vector<int>());
 #if defined(OS_CHROMEOS)
     LOG_IF(WARNING, !r) << "Sending zygote magic failed";
     // Exit normally on chromeos because session manager may send SIGTERM
@@ -489,8 +489,6 @@ base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle,
     // This is the child process.
 
     close(kZygoteSocketPairFd);  // Our socket from the browser.
-    if (UsingSUIDSandbox())
-      close(kZygoteIdFd);  // Another socket from the browser.
     base::GlobalDescriptors::GetInstance()->Reset(mapping);
 
     // Reset the process-wide command line to our new command line.
diff --git a/content/zygote/zygote_main_linux.cc b/content/zygote/zygote_main_linux.cc
index 0ec9b43..cc28a6f 100644
--- a/content/zygote/zygote_main_linux.cc
+++ b/content/zygote/zygote_main_linux.cc
@@ -23,6 +23,7 @@
 #include "base/linux_util.h"
 #include "base/native_library.h"
 #include "base/pickle.h"
+#include "base/posix/eintr_wrapper.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #include "base/rand_util.h"
 #include "base/sys_info.h"
@@ -344,19 +345,10 @@ static void ZygotePreSandboxInit() {
       new FontConfigIPC(GetSandboxFD()))->unref();
 }
 
-static void CloseFdAndHandleEintr(int fd) {
-  close(fd);
-}
-
 static bool CreateInitProcessReaper() {
-  // This "magic" socket must only appear in one process, so make sure
-  // it gets closed in the parent after fork().
-  base::Closure zygoteid_fd_closer =
-      base::Bind(CloseFdAndHandleEintr, kZygoteIdFd);
   // The current process becomes init(1), this function returns from a
   // newly created process.
-  const bool init_created =
-      sandbox::CreateInitProcessReaper(&zygoteid_fd_closer);
+  const bool init_created = sandbox::CreateInitProcessReaper(NULL);
   if (!init_created) {
     LOG(ERROR) << "Error creating an init process to reap zombies";
     return false;
@@ -460,6 +452,13 @@ bool ZygoteMain(const MainFunctionParams& params,
 
   const bool must_enable_setuid_sandbox =
       linux_sandbox->setuid_sandbox_client()->IsSuidSandboxChild();
+  if (must_enable_setuid_sandbox) {
+    // When we're launched through the setuid sandbox, ZygoteHostImpl::Init
+    // arranges for kZygoteIdFd to be a dummy file descriptor to satisfy an
+    // ancient setuid sandbox ABI requirement. However, the descriptor is no
+    // longer needed, so we can simply close it right away now.
+    CHECK_EQ(0, IGNORE_EINTR(close(kZygoteIdFd)));
+  }
 
   if (forkdelegate != NULL) {
     VLOG(1) << "ZygoteMain: initializing fork delegate";