Bug 1480401 - Avoid heap-allocated closures in async signal safe part of LaunchApp. r?froydnj
MozReview-Commit-ID: 4LYtBGbqtVh
--- 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;
};