Don't fork Zygote as a background process

sandbox/linux/suid/sandbox.c

 // 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.
 
 // http://code.google.com/p/chromium/wiki/LinuxSUIDSandbox
 
 #include "sandbox.h"
 
 #define _GNU_SOURCE
 #include <asm/unistd.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <sched.h>
 #include <signal.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/prctl.h>
 #include <sys/resource.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 #include "linux_util.h"
 #include "process_util.h"
 #include "suid_unsafe_environment_variables.h"
 
 #if !defined(CLONE_NEWPID)
 #define CLONE_NEWPID 0x20000000
 #endif
 #if !defined(CLONE_NEWNET)
 #define CLONE_NEWNET 0x40000000
 #endif
 
 static const char kSandboxDescriptorEnvironmentVarName[] = "SBX_D";
 static const char kSandboxHelperPidEnvironmentVarName[] = "SBX_HELPER_PID";
+// This number must be kept in sync in with common/zygote_commands_linux.h

[Jorge Lucangeli Obes, 2012/06/01 15:08:07]

"in with" -> "with"

+static const int kZygoteIdFd = 7;
 
 // These are the magic byte values which the sandboxed process uses to request
 // that it be chrooted.
 static const char kMsgChrootMe = 'C';
 static const char kMsgChrootSuccessful = 'O';
 
+static bool DropRoot();
+
+#define HANDLE_EINTR(x) TEMP_FAILURE_RETRY(x)
+
 static void FatalError(const char *msg, ...)
     __attribute__((noreturn, format(printf, 1, 2)));
 
 static void FatalError(const char *msg, ...) {
   va_list ap;
   va_start(ap, msg);
 
   vfprintf(stderr, msg, ap);
   fprintf(stderr, ": %s\n", strerror(errno));
   fflush(stderr);
+  va_end(ap);
   _exit(1);
 }
 
 // We will chroot() to the helper's /proc/self directory. Anything there will
 // not exist anymore if we make sure to wait() for the helper.
 //
 // /proc/self/fdinfo or /proc/self/fd are especially safe and will be empty
 // even if the helper survives as a zombie.
 //
 // There is very little reason to use fdinfo/ instead of fd/ but we are
@@ skipping 115 matching lines @@
 
   if (setenv(kSandboxHelperPidEnvironmentVarName, helper_pid_str, 1)) {
     perror("setenv");
     close(sv[1]);
     return false;
   }
 
   return true;
 }
 
+// Block until child_pid exits, then exit. Try to preserve the exit code.
+static void WaitForChildAndExit(pid_t child_pid) {
+  int exit_code = -1;
+  siginfo_t reaped_child_info;
+
+  int wait_ret =
+    HANDLE_EINTR(waitid(P_PID, child_pid, &reaped_child_info, WEXITED));
+
+  if (!wait_ret && reaped_child_info.si_pid == child_pid) {
+    if (reaped_child_info.si_code == CLD_EXITED) {
+      exit_code = reaped_child_info.si_status;
+    } else {
+      // Exit with code 0 if the child got signaled.
+      exit_code = 0;
+    }
+  }
+  _exit(exit_code);
+}
+
 static bool MoveToNewNamespaces() {
   // These are the sets of flags which we'll try, in order.
   const int kCloneExtraFlags[] = {
     CLONE_NEWPID | CLONE_NEWNET,
     CLONE_NEWPID,
   };
 
+  // We need to close kZygoteIdFd before the child can continue. We use this
+  // socketpair to tell the child when to continue;
+  int sync_fds[2];
+  if (socketpair(AF_UNIX, SOCK_STREAM, 0, sync_fds)) {
+    FatalError("Failed to create a socketpair");
+  }
+
   for (size_t i = 0;
        i < sizeof(kCloneExtraFlags) / sizeof(kCloneExtraFlags[0]);
        i++) {
     pid_t pid = syscall(__NR_clone, SIGCHLD | kCloneExtraFlags[i], 0, 0, 0);
 
-    if (pid > 0)
-      _exit(0);
+    if (pid > 0) {
+      if (DropRoot()) {

[agl, 2012/06/01 13:13:57]

Preferably exceptional conditions should be handled first, i.e.

if (!DropRoot()) {
  FatalError("Could not drop privileges")
}

and then the mainline code.

+        if (close(sync_fds[0]) || shutdown(sync_fds[1], SHUT_RD))
+          FatalError("Could not close socketpair");
+        // The kZygoteIdFd needs to be closed in the parent before
+        // Zygote gets started.
+        if (close(kZygoteIdFd))
+          FatalError("close");
+        // Tell our child to continue
+        if (send(sync_fds[1], "C", 1, MSG_NOSIGNAL) != 1)

[agl, 2012/06/01 13:13:57]

HANDLE_EINTR

+          FatalError("send");
+        if (close(sync_fds[1]))
+          FatalError("close");
+        // We want to keep a full process tree and we don't want our childs to
+        // be reparented to (the outer PID namespace) init. So we wait for it.
+        WaitForChildAndExit(pid);
+      } else {
+        FatalError("Could not drop privileges");
+      }
+      // NOTREACHED
+      FatalError("Not reached");
+    }
 
     if (pid == 0) {
+      if (close(sync_fds[1]) || shutdown(sync_fds[0], SHUT_WR))
+        FatalError("close ; shutdown");

[Jorge Lucangeli Obes, 2012/06/01 15:08:07]

The "close() || shutdown()" comments are not terribly consistent (one says
"Could not close..." and the other says "close; shutdown"

+
+      // Wait for the parent to confirm it closed kZygoteIdFd before we
+      // continue
+      char should_continue;
+      if (read(sync_fds[0], &should_continue, 1) != 1)

[agl, 2012/06/01 13:13:57]

HANDLE_EINTR

+        FatalError("Read on socketpair");
+      if (close(sync_fds[0]))
+        FatalError("close");
+
       if (kCloneExtraFlags[i] & CLONE_NEWPID) {
         setenv("SBX_PID_NS", "", 1 /* overwrite */);
       } else {
         unsetenv("SBX_PID_NS");
       }
 
       if (kCloneExtraFlags[i] & CLONE_NEWNET) {
         setenv("SBX_NET_NS", "", 1 /* overwrite */);
       } else {
         unsetenv("SBX_NET_NS");
@@ skipping 147 matching lines @@
   if (!DropRoot())
     return 1;
   if (!SetupChildEnvironment())
     return 1;
 
   execv(argv[1], &argv[1]);
   FatalError("execv failed");
 
   return 1;
 }