POSIX: don't allocate memory after forking.

base/process_util_posix.cc

Index: base/process_util_posix.cc
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index cca07dd24f139a72973306af8dcd1c8a7e2cc603..5f8a76c96742103a47003dcc9f58a71608419bdb 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -18,6 +18,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/debug_util.h"
+#include "base/dir_reader_posix.h"
 #include "base/eintr_wrapper.h"
 #include "base/logging.h"
 #include "base/platform_thread.h"
@@ -28,8 +29,13 @@
 #include "base/time.h"
 #include "base/waitable_event.h"
 
+
 #if defined(OS_MACOSX)
+#include <crt_externs.h>
+#define environ (*_NSGetEnviron())
 #include "base/mach_ipc_mac.h"
+#else
+extern char** environ;
 #endif
 
 const int kMicrosecondsPerSecond = 1000000;
@@ -173,24 +179,26 @@ class ScopedDIRClose {
 };
 typedef scoped_ptr_malloc<DIR, ScopedDIRClose> ScopedDIR;
 
-void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
 #if defined(OS_LINUX)
   static const rlim_t kSystemDefaultMaxFds = 8192;
-  static const char fd_dir[] = "/proc/self/fd";
+  static const char kFDDir[] = "/proc/self/fd";
 #elif defined(OS_MACOSX)
   static const rlim_t kSystemDefaultMaxFds = 256;
-  static const char fd_dir[] = "/dev/fd";
+  static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_SOLARIS)
   static const rlim_t kSystemDefaultMaxFds = 8192;
-  static const char fd_dir[] = "/dev/fd";
+  static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_FREEBSD)
   static const rlim_t kSystemDefaultMaxFds = 8192;
-  static const char fd_dir[] = "/dev/fd";
+  static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_OPENBSD)
   static const rlim_t kSystemDefaultMaxFds = 256;
-  static const char fd_dir[] = "/dev/fd";
+  static const char kFDDir[] = "/dev/fd";
 #endif
-  std::set<int> saved_fds;
+
+void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
+  // DANGER: no calls to malloc are allowed from now on:
+  // http://crbug.com/36678
 
   // Get the maximum number of FDs possible.
   struct rlimit nofile;
@@ -206,25 +214,20 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   if (max_fds > INT_MAX)
     max_fds = INT_MAX;
 
-  // Don't close stdin, stdout and stderr
-  saved_fds.insert(STDIN_FILENO);
-  saved_fds.insert(STDOUT_FILENO);
-  saved_fds.insert(STDERR_FILENO);
-
-  for (base::InjectiveMultimap::const_iterator
-       i = saved_mapping.begin(); i != saved_mapping.end(); ++i) {
-    saved_fds.insert(i->dest);
-  }
-
-  ScopedDIR dir_closer(opendir(fd_dir));
-  DIR *dir = dir_closer.get();
-  if (NULL == dir) {
-    DLOG(ERROR) << "Unable to open " << fd_dir;
+  DirReaderPosix fd_dir(kFDDir);
 
+  if (!fd_dir.IsValid()) {
     // Fallback case: Try every possible fd.
     for (rlim_t i = 0; i < max_fds; ++i) {
       const int fd = static_cast<int>(i);
-      if (saved_fds.find(fd) != saved_fds.end())
+      if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
+        continue;
+      InjectiveMultimap::const_iterator i;
+      for (i = saved_mapping.begin(); i != saved_mapping.end(); i++) {
+        if (fd == i->dest)
+          break;
+      }
+      if (i != saved_mapping.end())
         continue;
 
       // Since we're just trying to close anything we can find,
@@ -233,20 +236,27 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
     }
     return;
   }
-  int dir_fd = dirfd(dir);
 
-  struct dirent *ent;
-  while ((ent = readdir(dir))) {
+  const int dir_fd = fd_dir.fd();
+
+  for ( ; fd_dir.Next(); ) {
     // Skip . and .. entries.
-    if (ent->d_name[0] == '.')
+    if (fd_dir.name()[0] == '.')
       continue;
 
     char *endptr;
     errno = 0;
-    const long int fd = strtol(ent->d_name, &endptr, 10);
-    if (ent->d_name[0] == 0 || *endptr || fd < 0 || errno)
+    const long int fd = strtol(fd_dir.name(), &endptr, 10);
+    if (fd_dir.name()[0] == 0 || *endptr || fd < 0 || errno)
+      continue;
+    if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
       continue;
-    if (saved_fds.find(fd) != saved_fds.end())
+    InjectiveMultimap::const_iterator i;
+    for (i = saved_mapping.begin(); i != saved_mapping.end(); i++) {
+      if (fd == i->dest)
+        break;
+    }
+    if (i != saved_mapping.end())
       continue;
     if (fd == dir_fd)
       continue;
@@ -262,40 +272,6 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   }
 }
 
-// Sets all file descriptors to close on exec except for stdin, stdout
-// and stderr.
-// TODO(agl): Remove this function. It's fundamentally broken for multithreaded
-// apps.
-void SetAllFDsToCloseOnExec() {
-#if defined(OS_LINUX)
-  const char fd_dir[] = "/proc/self/fd";
-#elif defined(OS_MACOSX) || defined(OS_FREEBSD) || defined(OS_SOLARIS)
-  const char fd_dir[] = "/dev/fd";
-#endif
-  ScopedDIR dir_closer(opendir(fd_dir));
-  DIR *dir = dir_closer.get();
-  if (NULL == dir) {
-    DLOG(ERROR) << "Unable to open " << fd_dir;
-    return;
-  }
-
-  struct dirent *ent;
-  while ((ent = readdir(dir))) {
-    // Skip . and .. entries.
-    if (ent->d_name[0] == '.')
-      continue;
-    int i = atoi(ent->d_name);
-    // We don't close stdin, stdout or stderr.
-    if (i <= STDERR_FILENO)
-      continue;
-
-    int flags = fcntl(i, F_GETFD);
-    if ((flags == -1) || (fcntl(i, F_SETFD, flags | FD_CLOEXEC) == -1)) {
-      DLOG(ERROR) << "fcntl failure.";
-    }
-  }
-}
-
 #if defined(OS_MACOSX)
 static std::string MachErrorCode(kern_return_t err) {
   return StringPrintf("0x%x %s", err, mach_error_string(err));
@@ -357,21 +333,107 @@ static pid_t fork_and_get_task(task_t* child_task) {
 }
 
 bool LaunchApp(const std::vector<std::string>& argv,
-               const environment_vector& environ,
+               const environment_vector& env_changes,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle) {
   return LaunchAppAndGetTask(
-      argv, environ, fds_to_remap, wait, NULL, process_handle);
+      argv, env_changes, fds_to_remap, wait, NULL, process_handle);
 }
 #endif  // defined(OS_MACOSX)
 
+// AlterEnvironment returns a modified environment vector, constructed from the
+// current environment and the list of changes given in |changes|. Each key in
+// the environment is matched against the first element of the pairs. In the
+// event of a match, the value is replaced by the second of the pair, unless
+// the second is empty, in which case the key-value is removed.
+//
+// The returned array is allocated using new[] and must be freed by the caller.
+// Additionally, the first element is also allocated using new[] and should be
+// freed by the caller.
+static char** AlterEnvironment(const environment_vector& changes) {
+  unsigned count = 0;
+  unsigned size = 0;
+
+  for (unsigned i = 0; environ[i]; i++) {
+    const char *const pair = environ[i];
+    const char *const equals = strchr(pair, '=');
+    if (!equals) {
+      count++;
+      size += strlen(pair) + 1 /* terminating NUL */;
+      continue;
+    }
+    const unsigned keylen = equals - pair;
+    bool handled = false;
+    for (environment_vector::const_iterator
+         j = changes.begin(); j != changes.end(); j++) {
+      if (j->first.size() == keylen &&
+          memcmp(j->first.data(), pair, keylen) == 0) {
+        if (!j->second.empty()) {
+          count++;
+          size += keylen + 1 /* '=' */ + j->second.size() + 1 /* NUL */;
+        }
+        handled = true;
+        break;
+      }
+    }
+
+    if (!handled) {
+      count++;
+      size += strlen(pair) + 1 /* terminating NUL */;
+    }
+  }
+
+  char **const ret = new char*[count + 1 /* final NULL */];
+  unsigned k = 0;
+  char *scratch = new char[size];
+
+  for (unsigned i = 0; environ[i]; i++) {
+    const char *const pair = environ[i];
+    const char *const equals = strchr(pair, '=');
+    if (!equals) {
+      const unsigned len = strlen(pair);
+      ret[k++] = scratch;
+      memcpy(scratch, pair, len + 1);
+      scratch += len + 1;
+      continue;
+    }
+    const unsigned keylen = equals - pair;
+    bool handled = false;
+    for (environment_vector::const_iterator
+         j = changes.begin(); j != changes.end(); j++) {
+      if (j->first.size() == keylen &&
+          memcmp(j->first.data(), pair, keylen) == 0) {
+        if (!j->second.empty()) {
+          ret[k++] = scratch;
+          memcpy(scratch, pair, keylen + 1);
+          scratch += keylen + 1;
+          memcpy(scratch, j->second.c_str(), j->second.size() + 1);
+          scratch += j->second.size() + 1;
+        }
+        handled = true;
+        break;
+      }
+    }
+
+    if (!handled) {
+      const unsigned len = strlen(pair);
+      ret[k++] = scratch;
+      memcpy(scratch, pair, len + 1);
+      scratch += len + 1;
+    }
+  }
+
+  ret[k] = NULL;
+  return ret;
+}
+
 #if defined(OS_MACOSX)
 bool LaunchAppAndGetTask(
 #else
 bool LaunchApp(
 #endif
     const std::vector<std::string>& argv,
-    const environment_vector& environ,
+    const environment_vector& env_changes,
     const file_handle_mapping_vector& fds_to_remap,
     bool wait,
 #if defined(OS_MACOSX)
@@ -379,6 +441,15 @@ bool LaunchApp(
 #endif
     ProcessHandle* process_handle) {
   pid_t pid;
+  InjectiveMultimap fd_shuffle1, fd_shuffle2;
+  fd_shuffle1.reserve(fds_to_remap.size());
+  fd_shuffle2.reserve(fds_to_remap.size());
+  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
+  scoped_array<char*> new_environ(AlterEnvironment(env_changes));
+  // We need to free both the array of pointers, and the memory that they're
+  // all pointing into.
+  scoped_array<char> environ_buffer(new_environ[0]);
+
 #if defined(OS_MACOSX)
   if (task_handle == NULL) {
     pid = fork();
@@ -402,45 +473,44 @@ bool LaunchApp(
     RestoreDefaultExceptionHandler();
 #endif
 
-    InjectiveMultimap fd_shuffle;
+#if 0
+    // When debugging it can be helpful to check that we really aren't making
+    // any hidden calls to malloc.
+    void *malloc_thunk =
+        reinterpret_cast<void*>(reinterpret_cast<intptr_t>(malloc) & ~4095);
+    mprotect(malloc_thunk, 4096, PROT_READ | PROT_WRITE | PROT_EXEC);
+    memset(reinterpret_cast<void*>(malloc), 0xff, 8);
+#endif
+
+    // DANGER: no calls to malloc are allowed from now on:
+    // http://crbug.com/36678
+
     for (file_handle_mapping_vector::const_iterator
         it = fds_to_remap.begin(); it != fds_to_remap.end(); ++it) {
-      fd_shuffle.push_back(InjectionArc(it->first, it->second, false));
+      fd_shuffle1.push_back(InjectionArc(it->first, it->second, false));
+      fd_shuffle2.push_back(InjectionArc(it->first, it->second, false));
     }
 
-    for (environment_vector::const_iterator it = environ.begin();
-         it != environ.end(); ++it) {
-      if (it->first.empty())
-        continue;
-
-      if (it->second.empty()) {
-        unsetenv(it->first.c_str());
-      } else {
-        setenv(it->first.c_str(), it->second.c_str(), 1);
-      }
-    }
+    environ = new_environ.get();
 
     // Obscure fork() rule: in the child, if you don't end up doing exec*(),
     // you call _exit() instead of exit(). This is because _exit() does not
     // call any previously-registered (in the parent) exit handlers, which
     // might do things like block waiting for threads that don't even exist
     // in the child.
-    if (!ShuffleFileDescriptors(fd_shuffle))
-      _exit(127);
 
-    // If we are using the SUID sandbox, it sets a magic environment variable
-    // ("SBX_D"), so we remove that variable from the environment here on the
-    // off chance that it's already set.
-    unsetenv("SBX_D");
+    // fd_shuffle1 is mutated by this call because it cannot malloc.
+    if (!ShuffleFileDescriptors(&fd_shuffle1))
+      _exit(127);
 
-    CloseSuperfluousFds(fd_shuffle);
+    CloseSuperfluousFds(fd_shuffle2);
 
-    scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
     for (size_t i = 0; i < argv.size(); i++)
       argv_cstr[i] = const_cast<char*>(argv[i].c_str());
     argv_cstr[argv.size()] = NULL;
     execvp(argv_cstr[0], argv_cstr.get());
-    PLOG(ERROR) << "LaunchApp: execvp(" << argv_cstr[0] << ") failed";
+    RAW_LOG(ERROR, "LaunchApp: failed to execvp:");
+    RAW_LOG(ERROR, argv_cstr[0]);
     _exit(127);
   } else {
     // Parent process
@@ -633,6 +703,12 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
                                  bool do_search_path) {
   int pipe_fd[2];
   pid_t pid;
+  InjectiveMultimap fd_shuffle1, fd_shuffle2;
+  const std::vector<std::string>& argv = cl.argv();
+  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
+
+  fd_shuffle1.reserve(3);
+  fd_shuffle2.reserve(3);
 
   // Either |do_search_path| should be false or |envp| should be null, but not
   // both.
@@ -651,6 +727,8 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
 #if defined(OS_MACOSX)
         RestoreDefaultExceptionHandler();
 #endif
+        // DANGER: no calls to malloc are allowed from now on:
+        // http://crbug.com/36678
 
         // Obscure fork() rule: in the child, if you don't end up doing exec*(),
         // you call _exit() instead of exit(). This is because _exit() does not
@@ -661,18 +739,20 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
         if (dev_null < 0)
           _exit(127);
 
-        InjectiveMultimap fd_shuffle;
-        fd_shuffle.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
-        fd_shuffle.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
-        fd_shuffle.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
+        fd_shuffle1.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
+        fd_shuffle1.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
+        fd_shuffle1.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
+        // Adding another element here? Remeber to increase the argument to
+        // reserve(), above.
+
+        std::copy(fd_shuffle1.begin(), fd_shuffle1.end(),
+                  std::back_inserter(fd_shuffle2));
 
-        if (!ShuffleFileDescriptors(fd_shuffle))
+        if (!ShuffleFileDescriptors(&fd_shuffle1))
           _exit(127);
 
-        CloseSuperfluousFds(fd_shuffle);
+        CloseSuperfluousFds(fd_shuffle2);
 
-        const std::vector<std::string> argv = cl.argv();
-        scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
         for (size_t i = 0; i < argv.size(); i++)
           argv_cstr[i] = const_cast<char*>(argv[i].c_str());
         argv_cstr[argv.size()] = NULL;