Bug 622992 - Update Chrome IPC to not allocate memory after forking. r=cjones

ipc/chromium/src/base/dir_reader_fallback.h

@@ -0,0 +1,31 @@
+// Copyright (c) 2010 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.
+
+#ifndef BASE_DIR_READER_FALLBACK_H_
+#define BASE_DIR_READER_FALLBACK_H_
+#pragma once
+
+namespace base {
+
+class DirReaderFallback {
+ public:
+  // Open a directory. If |IsValid| is true, then |Next| can be called to start
+  // the iteration at the beginning of the directory.
+  explicit DirReaderFallback(const char* directory_path) { }
+  // After construction, IsValid returns true iff the directory was
+  // successfully opened.
+  bool IsValid() const { return false; }
+  // Move to the next entry returning false if the iteration is complete.
+  bool Next() { return false; }
+  // Return the name of the current directory entry.
+  const char* name() { return 0;}
+  // Return the file descriptor which is being used.
+  int fd() const { return -1; }
+  // Returns true if this is a no-op fallback class (for testing).
+  static bool IsFallback() { return true; }
+};
+
+}  // namespace base
+
+#endif  // BASE_DIR_READER_FALLBACK_H_

ipc/chromium/src/base/dir_reader_linux.h

@@ -0,0 +1,99 @@
+// Copyright (c) 2010 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.
+
+#ifndef BASE_DIR_READER_LINUX_H_
+#define BASE_DIR_READER_LINUX_H_
+#pragma once
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "base/logging.h"
+#include "base/eintr_wrapper.h"
+
+// See the comments in dir_reader_posix.h about this.
+
+namespace base {
+
+struct linux_dirent {
+  uint64_t        d_ino;
+  int64_t         d_off;
+  unsigned short  d_reclen;
+  unsigned char   d_type;
+  char            d_name[0];
+};
+
+class DirReaderLinux {
+ public:
+  explicit DirReaderLinux(const char* directory_path)
+      : fd_(open(directory_path, O_RDONLY | O_DIRECTORY)),
+        offset_(0),
+        size_(0) {
+    memset(buf_, 0, sizeof(buf_));
+  }
+
+  ~DirReaderLinux() {
+    if (fd_ >= 0) {
+      if (HANDLE_EINTR(close(fd_)))
+        DLOG(ERROR) << "Failed to close directory handle";
+    }
+  }
+
+  bool IsValid() const {
+    return fd_ >= 0;
+  }
+
+  // Move to the next entry returning false if the iteration is complete.
+  bool Next() {
+    if (size_) {
+      linux_dirent* dirent = reinterpret_cast<linux_dirent*>(&buf_[offset_]);
+      offset_ += dirent->d_reclen;
+    }
+
+    if (offset_ != size_)
+      return true;
+
+    const int r = syscall(__NR_getdents64, fd_, buf_, sizeof(buf_));
+    if (r == 0)
+      return false;
+    if (r == -1) {
+      DLOG(ERROR) << "getdents64 returned an error: " << errno;
+      return false;
+    }
+    size_ = r;
+    offset_ = 0;
+    return true;
+  }
+
+  const char* name() const {
+    if (!size_)
+      return NULL;
+
+    const linux_dirent* dirent =
+        reinterpret_cast<const linux_dirent*>(&buf_[offset_]);
+    return dirent->d_name;
+  }
+
+  int fd() const {
+    return fd_;
+  }
+
+  static bool IsFallback() {
+    return false;
+  }
+
+ private:
+  const int fd_;
+  unsigned char buf_[512];
+  size_t offset_, size_;
+
+  DISALLOW_COPY_AND_ASSIGN(DirReaderLinux);
+};
+
+}  // namespace base
+
+#endif // BASE_DIR_READER_LINUX_H_

ipc/chromium/src/base/dir_reader_posix.h

@@ -0,0 +1,37 @@
+// Copyright (c) 2010 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.
+
+#ifndef BASE_DIR_READER_POSIX_H_
+#define BASE_DIR_READER_POSIX_H_
+#pragma once
+
+#include "build/build_config.h"
+
+// This header provides a class, DirReaderPosix, which allows one to open and
+// read from directories without allocating memory. For the interface, see
+// the generic fallback in dir_reader_fallback.h.
+
+// Mac note: OS X has getdirentries, but it only works if we restrict Chrome to
+// 32-bit inodes. There is a getdirentries64 syscall in 10.6, but it's not
+// wrapped and the direct syscall interface is unstable. Using an unstable API
+// seems worse than falling back to enumerating all file descriptors so we will
+// probably never implement this on the Mac.
+
+#if defined(OS_LINUX)
+#include "base/dir_reader_linux.h"
+#else
+#include "base/dir_reader_fallback.h"
+#endif
+
+namespace base {
+
+#if defined(OS_LINUX)
+typedef DirReaderLinux DirReaderPosix;
+#else
+typedef DirReaderFallback DirReaderPosix;
+#endif
+
+}  // namespace base
+
+#endif // BASE_DIR_READER_POSIX_H_

ipc/chromium/src/base/file_descriptor_shuffle.cc

@@ -12,26 +12,36 @@
 
 namespace base {
 
-bool PerformInjectiveMultimap(const InjectiveMultimap& m_in,
+bool PerformInjectiveMultimapDestructive(
-                              InjectionDelegate* delegate) {
+    InjectiveMultimap* m, InjectionDelegate* delegate) {
-  InjectiveMultimap m(m_in);
+  static const size_t kMaxExtraFDs = 16;
-  std::vector<int> extra_fds;
+  int extra_fds[kMaxExtraFDs];
+  unsigned next_extra_fd = 0;
 
-  for (InjectiveMultimap::iterator i = m.begin(); i != m.end(); ++i) {
+  // DANGER: this function may not allocate.
+
+  for (InjectiveMultimap::iterator i = m->begin(); i != m->end(); ++i) {
     int temp_fd = -1;
 
     // We DCHECK the injectiveness of the mapping.
-    for (InjectiveMultimap::iterator j = i + 1; j != m.end(); ++j)
+    for (InjectiveMultimap::iterator j = i + 1; j != m->end(); ++j) {
-      DCHECK(i->dest != j->dest);
+      DCHECK(i->dest != j->dest) << "Both fd " << i->source
+          << " and " << j->source << " map to " << i->dest;
+    }
 
     const bool is_identity = i->source == i->dest;
 
-    for (InjectiveMultimap::iterator j = i + 1; j != m.end(); ++j) {
+    for (InjectiveMultimap::iterator j = i + 1; j != m->end(); ++j) {
       if (!is_identity && i->dest == j->source) {
         if (temp_fd == -1) {
           if (!delegate->Duplicate(&temp_fd, i->dest))
             return false;
-          extra_fds.push_back(temp_fd);
+          if (next_extra_fd < kMaxExtraFDs) {
+            extra_fds[next_extra_fd++] = temp_fd;
+          } else {
+              DLOG(ERROR) << "PerformInjectiveMultimapDestructive overflowed "
+                          << "extra_fds. Leaking file descriptors!";
+          }
         }
 
         j->source = temp_fd;
@@ -56,14 +66,18 @@ bool PerformInjectiveMultimap(const InjectiveMultimap& m_in,
       delegate->Close(i->source);
   }
 
-  for (std::vector<int>::const_iterator
+  for (unsigned i = 0; i < next_extra_fd; i++)
-       i = extra_fds.begin(); i != extra_fds.end(); ++i) {
+    delegate->Close(extra_fds[i]);
-    delegate->Close(*i);
-  }
 
   return true;
 }
 
+bool PerformInjectiveMultimap(const InjectiveMultimap& m_in,
+                              InjectionDelegate* delegate) {
+    InjectiveMultimap m(m_in);
+    return PerformInjectiveMultimapDestructive(&m, delegate);
+}
+
 bool FileDescriptorTableInjection::Duplicate(int* result, int fd) {
   *result = HANDLE_EINTR(dup(fd));
   return *result >= 0;

ipc/chromium/src/base/file_descriptor_shuffle.h

@@ -65,10 +65,13 @@ typedef std::vector<InjectionArc> InjectiveMultimap;
 
 bool PerformInjectiveMultimap(const InjectiveMultimap& map,
                               InjectionDelegate* delegate);
+bool PerformInjectiveMultimapDestructive(InjectiveMultimap* map,
+                                         InjectionDelegate* delegate);
 
-static inline bool ShuffleFileDescriptors(const InjectiveMultimap& map) {
+// This function will not call malloc but will mutate |map|
+static inline bool ShuffleFileDescriptors(InjectiveMultimap *map) {
   FileDescriptorTableInjection delegate;
-  return PerformInjectiveMultimap(map, &delegate);
+  return PerformInjectiveMultimapDestructive(map, &delegate);
 }
 
 }  // namespace base

ipc/chromium/src/base/process_util_linux.cc

@@ -48,6 +48,12 @@ bool LaunchApp(const std::vector<std::string>& argv,
                const environment_map& env_vars_to_set,
                bool wait, ProcessHandle* process_handle,
                ProcessArchitecture arch) {
+  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
+  // Illegal to allocate memory after fork and before execvp
+  InjectiveMultimap fd_shuffle1, fd_shuffle2;
+  fd_shuffle1.reserve(fds_to_remap.size());
+  fd_shuffle2.reserve(fds_to_remap.size());
+
 #ifdef MOZ_MEMORY_ANDROID
   /* We specifically don't call pthread_atfork in jemalloc because it is not
     available in bionic until 2.3. However without it, jemalloc could
@@ -63,24 +69,23 @@ bool LaunchApp(const std::vector<std::string>& argv,
     return false;
 
   if (pid == 0) {
-    InjectiveMultimap fd_shuffle;
     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));
     }
 
-    if (!ShuffleFileDescriptors(fd_shuffle))
+    if (!ShuffleFileDescriptors(&fd_shuffle1))
-      exit(127);
+      _exit(127);
 
-    CloseSuperfluousFds(fd_shuffle);
+    CloseSuperfluousFds(fd_shuffle2);
 
     for (environment_map::const_iterator it = env_vars_to_set.begin();
          it != env_vars_to_set.end(); ++it) {
       if (setenv(it->first.c_str(), it->second.c_str(), 1/*overwrite*/))
-        exit(127);
+        _exit(127);
     }
 
-    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;

ipc/chromium/src/base/process_util_posix.cc

@@ -1,3 +1,4 @@
+//* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 // Copyright (c) 2006-2008 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.
@@ -25,6 +26,7 @@
 #include "base/sys_info.h"
 #include "base/time.h"
 #include "base/waitable_event.h"
+#include "base/dir_reader_posix.h"
 
 const int kMicrosecondsPerSecond = 1000000;
 
@@ -86,6 +88,10 @@ bool KillProcess(ProcessHandle process_id, int exit_code, bool wait) {
   return result;
 }
 
+#ifdef ANDROID
+typedef unsigned long int rlim_t;
+#endif
+
 // A class to handle auto-closing of DIR*'s.
 class ScopedDIRClose {
  public:
@@ -97,19 +103,20 @@ class ScopedDIRClose {
 };
 typedef scoped_ptr_malloc<DIR, ScopedDIRClose> ScopedDIR;
 
-#ifdef ANDROID
-typedef unsigned long int rlim_t;
-#endif
 
 void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
-#if defined(OS_LINUX)
+  // DANGER: no calls to malloc are allowed from now on:
+  // http://crbug.com/36678
+#if defined(ANDROID)
+  static const rlim_t kSystemDefaultMaxFds = 1024;
+  static const char kFDDir[] = "/proc/self/fd";
+#elif 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";
 #endif
-  std::set<int> saved_fds;
 
   // Get the maximum number of FDs possible.
   struct rlimit nofile;
@@ -125,52 +132,63 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   if (max_fds > INT_MAX)
     max_fds = INT_MAX;
 
-  // Don't close stdin, stdout and stderr
+  DirReaderPosix fd_dir(kFDDir);
-  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;
 
+  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 j;
+      for (j = saved_mapping.begin(); j != saved_mapping.end(); j++) {
+        if (fd == j->dest)
+          break;
+      }
+      if (j != saved_mapping.end())
         continue;
 
+      // Since we're just trying to close anything we can find,
+      // ignore any error return values of close().
       HANDLE_EINTR(close(fd));
     }
     return;
   }
 
-  struct dirent *ent;
+  const int dir_fd = fd_dir.fd();
-  while ((ent = readdir(dir))) {
+
+  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);
+    const long int fd = strtol(fd_dir.name(), &endptr, 10);
-    if (ent->d_name[0] == 0 || *endptr || fd < 0 || errno)
+    if (fd_dir.name()[0] == 0 || *endptr || fd < 0 || errno)
       continue;
-    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;
+    if (fd == dir_fd)
       continue;
 
     // When running under Valgrind, Valgrind opens several FDs for its
     // own use and will complain if we try to close them.  All of
     // these FDs are >= |max_fds|, so we can check against that here
     // before closing.  See https://bugs.kde.org/show_bug.cgi?id=191758
-    if (fd < static_cast<int>(max_fds))
+    if (fd < static_cast<int>(max_fds)) {
-      HANDLE_EINTR(close(fd));
+      int ret = HANDLE_EINTR(close(fd));
+      if (ret != 0) {
+        DLOG(ERROR) << "Problem closing fd";
+      }
+    }
   }
 }
 
@@ -419,6 +437,13 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) {
   int pipe_fd[2];
   pid_t pid;
 
+  // Illegal to allocate memory after fork and before execvp
+  InjectiveMultimap fd_shuffle1, fd_shuffle2;
+  fd_shuffle1.reserve(3);
+  fd_shuffle2.reserve(3);
+  const std::vector<std::string>& argv = cl.argv();
+  scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
+
   if (pipe(pipe_fd) < 0)
     return false;
 
@@ -429,27 +454,35 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) {
       return false;
     case 0:  // child
       {
+        // 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.
         int dev_null = open("/dev/null", O_WRONLY);
         if (dev_null < 0)
-          exit(127);
+          _exit(127);
 
-        InjectiveMultimap fd_shuffle;
+        fd_shuffle1.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
-        fd_shuffle.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
+        fd_shuffle1.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
-        fd_shuffle.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
+        fd_shuffle1.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
-        fd_shuffle.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
+        // Adding another element here? Remeber to increase the argument to
+        // reserve(), above.
 
-        if (!ShuffleFileDescriptors(fd_shuffle))
+        std::copy(fd_shuffle1.begin(), fd_shuffle1.end(),
-          exit(127);
+                  std::back_inserter(fd_shuffle2));
 
-        CloseSuperfluousFds(fd_shuffle);
+        // fd_shuffle1 is mutated by this call because it cannot malloc.
+        if (!ShuffleFileDescriptors(&fd_shuffle1))
+          _exit(127);
+
+        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;
         execvp(argv_cstr[0], argv_cstr.get());
-        exit(127);
+        _exit(127);
       }
     default:  // parent
       {