Bug 1480401 - Avoid heap-allocated closures in async signal safe part of LaunchApp. r=froydnj
authorJed Davis <jld@mozilla.com>
Thu, 02 Aug 2018 14:18:01 -0600
changeset 430152 1e65391c7583a32fdefea325dc37b476cfbe2b5c
parent 430151 b70410825f1e4aaad805869795e6dc6abfeadf20
child 430153 25c8ddd2fe538d8765e260af4eb8a9494e22c809
push id34384
push userdvarga@mozilla.com
push dateSat, 04 Aug 2018 21:44:57 +0000
treeherdermozilla-central@073983e062f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1480401
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1480401 - Avoid heap-allocated closures in async signal safe part of LaunchApp. r=froydnj MozReview-Commit-ID: 4LYtBGbqtVh
ipc/chromium/src/base/process_util.h
ipc/chromium/src/base/process_util_linux.cc
ipc/glue/FileDescriptorShuffle.h
--- a/ipc/chromium/src/base/process_util.h
+++ b/ipc/chromium/src/base/process_util.h
@@ -123,16 +123,17 @@ struct LaunchOptions {
 
 #if defined(OS_LINUX) || defined(OS_SOLARIS)
   struct ForkDelegate {
     virtual ~ForkDelegate() { }
     virtual pid_t Fork() = 0;
   };
 
   // If non-null, the fork delegate will be called instead of fork().
+  // It is not required to call pthread_atfork hooks.
   mozilla::UniquePtr<ForkDelegate> fork_delegate = nullptr;
 #endif
 };
 
 #if defined(OS_WIN)
 // Runs the given application name with the given command line. Normally, the
 // first command line argument should be the path to the process, and don't
 // forget to quote it.
--- a/ipc/chromium/src/base/process_util_linux.cc
+++ b/ipc/chromium/src/base/process_util_linux.cc
@@ -32,31 +32,39 @@ bool LaunchApp(const std::vector<std::st
 
   EnvironmentArray envp = BuildEnvironmentArray(options.env_map);
   mozilla::ipc::FileDescriptorShuffle shuffle;
   if (!shuffle.Init(options.fds_to_remap)) {
     return false;
   }
 
   pid_t pid = options.fork_delegate ? options.fork_delegate->Fork() : fork();
+  // WARNING: if pid == 0, only async signal safe operations are permitted from
+  // here until exec or _exit.
+  //
+  // Specifically, heap allocation is not safe: the sandbox's fork substitute
+  // won't run the pthread_atfork handlers that fix up the malloc locks.
+
   if (pid < 0)
     return false;
 
   if (pid == 0) {
     // In the child:
     for (const auto& fds : shuffle.Dup2Sequence()) {
       if (HANDLE_EINTR(dup2(fds.first, fds.second)) != fds.second) {
         // This shouldn't happen, but check for it.  And see below
         // about logging being unsafe here, so this is debug only.
         DLOG(ERROR) << "dup2 failed";
         _exit(127);
       }
     }
 
-    CloseSuperfluousFds(shuffle.MapsToFunc());
+    auto fdIsUsed = [&shuffle](int fd) { return shuffle.MapsTo(fd); };
+    // Constructing a std::function from a reference_wrapper won't allocate.
+    CloseSuperfluousFds(std::ref(fdIsUsed));
 
     for (size_t i = 0; i < argv.size(); i++)
       argv_cstr[i] = const_cast<char*>(argv[i].c_str());
     argv_cstr[argv.size()] = NULL;
 
     execve(argv_cstr[0], argv_cstr.get(), envp.get());
     // if we get here, we're in serious trouble and should complain loudly
     // NOTE: This is async signal unsafe; it could deadlock instead.  (But
--- a/ipc/glue/FileDescriptorShuffle.h
+++ b/ipc/glue/FileDescriptorShuffle.h
@@ -45,22 +45,16 @@ public:
   // Accessor for the dup2() sequence.  Do not use the returned value
   // or the fds contained in it after this object is destroyed.
   MappingRef Dup2Sequence() const { return mMapping; }
 
   // Tests whether the given fd is used as a destination in this mapping.
   // Can be used to close other fds after performing the dup2()s.
   bool MapsTo(int aFd) const;
 
-  // Wraps MapsTo in a function object, as a convenience for use with
-  // base::CloseSuperfluousFds.
-  std::function<bool(int)> MapsToFunc() const {
-    return [this](int fd) { return MapsTo(fd); };
-  }
-
 private:
   nsTArray<std::pair<int, int>> mMapping;
   nsTArray<int> mTempFds;
   int mMaxDst;
 
   FileDescriptorShuffle(const FileDescriptorShuffle&) = delete;
   void operator=(const FileDescriptorShuffle&) = delete;
 };