Start all children in their own PID namespace.

content/zygote/zygote_linux.cc

Index: content/zygote/zygote_linux.cc
diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc
index 5944f87aa1258dff7a908eb6d5a2d55cea93a05f..1a3fe6c889aff224c8c03fe5ab0c4c402b42495d 100644
--- a/content/zygote/zygote_linux.cc
+++ b/content/zygote/zygote_linux.cc
@@ -36,6 +36,7 @@
 #include "content/public/common/zygote_fork_delegate_linux.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_switches.h"
+#include "sandbox/linux/services/credentials.h"
 
 // See http://code.google.com/p/chromium/wiki/LinuxZygote
 
@@ -47,6 +48,20 @@ namespace {
 void SIGCHLDHandler(int signal) {
 }
 
+// On Linux, when a process is the init process of a PID namespace, it cannot be
+// terminated by signals like SIGTERM or SIGINT, since they are ignored unless
+// we register a handler for them. In the handlers, we exit with this special
+// exit code that GetTerminationStatus understands to mean that we were
+// terminated by an external signal.
+const int kKilledExitCode = 0x80;
+
+void TerminationSignalHandler(int signal) {
+  // Return a special exit code so that the process is detected as terminated by
+  // a signal. We cannot terminate ourself with a signal since we may be the
+  // init process in a PID namespace.
+  _exit(kKilledExitCode);
+}
+
 int LookUpFd(const base::GlobalDescriptors::Mapping& fd_mapping, uint32_t key) {
   for (size_t index = 0; index < fd_mapping.size(); ++index) {
     if (fd_mapping[index].key == key)
@@ -275,12 +290,10 @@ bool Zygote::GetTerminationStatus(base::ProcessHandle real_pid,
                                   bool known_dead,
                                   base::TerminationStatus* status,
                                   int* exit_code) {
-
   ZygoteProcessInfo child_info;
   if (!GetProcessInfo(real_pid, &child_info)) {
-    LOG(ERROR) << "Zygote::GetTerminationStatus for unknown PID "
+    LOG(FATAL) << "Zygote::GetTerminationStatus for unknown PID "
                << real_pid;
-    NOTREACHED();
     return false;
   }
   // We know about |real_pid|.
@@ -305,6 +318,11 @@ bool Zygote::GetTerminationStatus(base::ProcessHandle real_pid,
     // Time to forget about this process.
     process_info_map_.erase(real_pid);
   }
+
+  if (WIFEXITED(*exit_code) && WEXITSTATUS(*exit_code) == kKilledExitCode) {
+    *status = base::TERMINATION_STATUS_PROCESS_WAS_KILLED;
+  }
+
   return true;
 }
 
@@ -375,12 +393,30 @@ int Zygote::ForkWithRealPid(const std::string& process_type,
     CHECK_NE(pid, 0);
   } else {
     CreatePipe(&read_pipe, &write_pipe);
-    // This is roughly equivalent to a fork(). We are using ForkWithFlags mainly
-    // to give it some more diverse test coverage.
-    pid = base::ForkWithFlags(SIGCHLD, nullptr, nullptr);
+    int clone_flags = SIGCHLD;
+    if (sandbox_flags_ & kSandboxLinuxPIDNS &&
+        sandbox_flags_ & kSandboxLinuxUserNS) {
+      clone_flags |= CLONE_NEWPID;

[jln (very slow on Chromium), 2015/02/25 21:32:48]

I know there was some back and forth on this, but how about not supporting
kernel that don't enable PID namespace?

It's probably better to be forceful now while everything is new rather than drag
a ton of complexity down the line...

[rickyz (no longer on Chrome), 2015/03/21 01:35:31]

I'm happy to not go out of my way to support people with userns and no pidns,
but I don't think it really adds much complexity in this case (I think it'll
probably work fine with this code, and we won't have to bend over backwards to
make it work).

+    }
+    pid = base::ForkWithFlags(clone_flags, nullptr, nullptr);
   }
 
   if (pid == 0) {
+    if (sandbox::Credentials::HasAnyCapability()) {
+      CHECK(sandbox::Credentials::DropAllCapabilities(

[jln (very slow on Chromium), 2015/02/25 21:32:48]

Why not always drop capabilities?

Is it because proc_fd_ may not be available if --no-sandbox?

I wanted to send a change to initialize proc_fd_ in the constructor (and make it
a ScopedFD). Would that be sufficient here?

Even without that, I would rather have something like:

if (proc_fd >= 0) {
  sandbox::Credentials::DropAllCapabilities(proc_fd);
} else {
  sandbox::Credentials::DropAllCapabilities();
}

[rickyz (no longer on Chrome), 2015/03/21 01:35:31]

Yeah, that's exactly the reason. I haven't addressed this comment yet, but will
in the next patchset.

+          LinuxSandbox::GetInstance()->proc_fd()));
+    }
+
+    // If the process is the init process inside a PID namespace, it must have

[jln (very slow on Chromium), 2015/02/25 21:32:48]

(I'm trying to figure out how we can re-factor the whole sandbox business into a
nice generic sandbox class)

But even before that, this part should be exposed as an API, probably in
NamespaceSandbox.

1. Something along the lines of AssumePidNameSpaceOwnership() (with clear
documentation and some unit testing).

It would:

- DCHECK(1 == getpid);
- Drop all capabilities
- Install signal handlers for all signals that can be caught and have a default
"TERM" action.

Note: you should handle SIGHUP, SIGUSER1, SIGUSER2, SIGPIPE...

2. Provide a small, trivial ForkInNewPidNameSpace() wrapper.

(It's useful because it's a good place for documenting that children should
either use CreateInitProcessReaper() or AssumePidNameSpaceOwnership() and not
fork again).

3. Provide an API there along the line of GetTerminationStatus() that can be
used to retreive termination status for processes started with
ForkInNewPidNameSpace().

[rickyz (no longer on Chrome), 2015/03/21 01:35:31]

Ugh, this signal handling stuff is so horrible. Here is an attempt at one way to
expose the right API. I didn't drop capabilities in ForkWithNewPidNamespace in
case the caller wants to fork a new pid namespace inside. Also, I made it the
caller's responsibility to call InstallTerminationSignalHandler for any signals
that they want to terminate so that they can register different exit codes for
different signals. I'm really not happy with the state of this though, so any
additional suggestions on how to make this nicer are more than appreciated!

+    // explicit SIGTERM and SIGINT handlers.
+    if (getpid() == 1) {
+      struct sigaction action;
+      memset(&action, 0, sizeof(action));
+      action.sa_handler = &TerminationSignalHandler;
+      PCHECK(sigaction(SIGINT, &action, nullptr) == 0);
+      PCHECK(sigaction(SIGTERM, &action, nullptr) == 0);
+    }
+
     // In the child process.
     write_pipe.reset();