From ccef2eb783c650835ede55fa3af7b5cced3d63ec Mon Sep 17 00:00:00 2001 From: Josh Aas Date: Fri, 10 Sep 2010 13:09:30 -0400 Subject: [PATCH] Bug 592951: Use 'posix_spawnp' to launch child processes on Mac OS X. r=ted a=blocking-b6+ --- ipc/app/MozillaRuntimeMain.cpp | 32 +-- ipc/chromium/src/base/process_util.h | 19 -- ipc/chromium/src/base/process_util_mac.mm | 256 ++++++++-------------- ipc/glue/GeckoChildProcessHost.cpp | 68 +++++- toolkit/xre/nsEmbedFunctions.cpp | 74 +++++++ 5 files changed, 228 insertions(+), 221 deletions(-) diff --git a/ipc/app/MozillaRuntimeMain.cpp b/ipc/app/MozillaRuntimeMain.cpp index 61c988b9361..d2db009aa9a 100644 --- a/ipc/app/MozillaRuntimeMain.cpp +++ b/ipc/app/MozillaRuntimeMain.cpp @@ -54,37 +54,17 @@ int main(int argc, char* argv[]) { -#if defined(MOZ_CRASHREPORTER) - if (argc < 2) - return 1; - const char* const crashReporterArg = argv[--argc]; - -# if defined(XP_WIN) || defined(XP_MACOSX) - // on windows and mac, |crashReporterArg| is the named pipe on which the - // server is listening for requests, or "-" if crash reporting is - // disabled. - if (0 != strcmp("-", crashReporterArg) - && !XRE_SetRemoteExceptionHandler(crashReporterArg)) - return 1; -# elif defined(OS_LINUX) - // on POSIX, |crashReporterArg| is "true" if crash reporting is - // enabled, false otherwise - if (0 != strcmp("false", crashReporterArg) - && !XRE_SetRemoteExceptionHandler(NULL)) - return 1; -# else -# error "OOP crash reporting unsupported on this platform" -# endif -#endif // if defined(MOZ_CRASHREPORTER) - #if defined(XP_WIN) && defined(DEBUG_bent) MessageBox(NULL, L"Hi", L"Hi", MB_OK); #endif - GeckoProcessType proctype = - XRE_StringToChildProcessType(argv[argc - 1]); + // Check for the absolute minimum number of args we need to move + // forward here. We expect the last arg to be the child process type. + if (argc < 1) + return 1; + GeckoProcessType proctype = XRE_StringToChildProcessType(argv[--argc]); - nsresult rv = XRE_InitChildProcess(argc - 1, argv, proctype); + nsresult rv = XRE_InitChildProcess(argc, argv, proctype); NS_ENSURE_SUCCESS(rv, 1); return 0; diff --git a/ipc/chromium/src/base/process_util.h b/ipc/chromium/src/base/process_util.h index 851a4f3cac1..029d3faf896 100644 --- a/ipc/chromium/src/base/process_util.h +++ b/ipc/chromium/src/base/process_util.h @@ -131,36 +131,17 @@ bool LaunchApp(const std::wstring& cmdline, // // Note that the first argument in argv must point to the filename, // and must be fully specified. -#ifdef OS_MACOSX -typedef std::vector > file_handle_mapping_vector; -bool LaunchApp(const std::vector& argv, - const file_handle_mapping_vector& fds_to_remap, - bool wait, ProcessHandle* process_handle, - task_t* task_handle); - -#if defined(OS_LINUX) || defined(OS_MACOSX) -typedef std::map environment_map; -bool LaunchApp(const std::vector& argv, - const file_handle_mapping_vector& fds_to_remap, - const environment_map& env_vars_to_set, - bool wait, ProcessHandle* process_handle, - task_t* task_handle); -#endif -#else // !OS_MACOSX typedef std::vector > file_handle_mapping_vector; bool LaunchApp(const std::vector& argv, const file_handle_mapping_vector& fds_to_remap, bool wait, ProcessHandle* process_handle); -#if defined(OS_LINUX) || defined(OS_MACOSX) typedef std::map environment_map; bool LaunchApp(const std::vector& argv, const file_handle_mapping_vector& fds_to_remap, const environment_map& env_vars_to_set, bool wait, ProcessHandle* process_handle); #endif -#endif -#endif // Executes the application specified by cl. This function delegates to one // of the above two platform-specific functions. diff --git a/ipc/chromium/src/base/process_util_mac.mm b/ipc/chromium/src/base/process_util_mac.mm index 183755a8b80..647b5f841f4 100644 --- a/ipc/chromium/src/base/process_util_mac.mm +++ b/ipc/chromium/src/base/process_util_mac.mm @@ -17,192 +17,114 @@ #include "base/eintr_wrapper.h" #include "base/logging.h" #include "base/rand_util.h" -#include "base/scoped_ptr.h" #include "base/string_util.h" #include "base/time.h" -#include "chrome/common/mach_ipc_mac.h" namespace base { -static std::string MachErrorCode(kern_return_t err) { - return StringPrintf("0x%x %s", err, mach_error_string(err)); -} - -// Forks the current process and returns the child's |task_t| in the parent -// process. -static pid_t fork_and_get_task(task_t* child_task) { - const int kTimeoutMs = 100; - kern_return_t err; - - // Put a random number into the channel name, so that a compromised renderer - // can't pretend being the child that's forked off. - std::string mach_connection_name = StringPrintf( - "org.mozilla.samplingfork.%p.%d", - child_task, base::RandInt(0, std::numeric_limits::max())); - ReceivePort parent_recv_port(mach_connection_name.c_str()); - - // Error handling philosophy: If Mach IPC fails, don't touch |child_task| but - // return a valid pid. If IPC fails in the child, the parent will have to wait - // until kTimeoutMs is over. This is not optimal, but I've never seen it - // happen, and stuff should still mostly work. - pid_t pid = fork(); - switch (pid) { - case -1: - return pid; - case 0: { // child - ReceivePort child_recv_port; - - MachSendMessage child_message(/* id= */0); - if (!child_message.AddDescriptor(mach_task_self())) { - LOG(ERROR) << "child AddDescriptor(mach_task_self()) failed."; - return pid; - } - mach_port_t raw_child_recv_port = child_recv_port.GetPort(); - if (!child_message.AddDescriptor(raw_child_recv_port)) { - LOG(ERROR) << "child AddDescriptor(" << raw_child_recv_port - << ") failed."; - return pid; - } - - MachPortSender child_sender(mach_connection_name.c_str()); - err = child_sender.SendMessage(child_message, kTimeoutMs); - if (err != KERN_SUCCESS) { - LOG(ERROR) << "child SendMessage() failed: " << MachErrorCode(err); - return pid; - } - - MachReceiveMessage parent_message; - err = child_recv_port.WaitForMessage(&parent_message, kTimeoutMs); - if (err != KERN_SUCCESS) { - LOG(ERROR) << "child WaitForMessage() failed: " << MachErrorCode(err); - return pid; - } - - if (parent_message.GetTranslatedPort(0) == MACH_PORT_NULL) { - LOG(ERROR) << "child GetTranslatedPort(0) failed."; - return pid; - } - err = task_set_bootstrap_port(mach_task_self(), - parent_message.GetTranslatedPort(0)); - if (err != KERN_SUCCESS) { - LOG(ERROR) << "child task_set_bootstrap_port() failed: " - << MachErrorCode(err); - return pid; - } - break; - } - default: { // parent - MachReceiveMessage child_message; - err = parent_recv_port.WaitForMessage(&child_message, kTimeoutMs); - if (err != KERN_SUCCESS) { - LOG(ERROR) << "parent WaitForMessage() failed: " << MachErrorCode(err); - return pid; - } - - if (child_message.GetTranslatedPort(0) == MACH_PORT_NULL) { - LOG(ERROR) << "parent GetTranslatedPort(0) failed."; - return pid; - } - *child_task = child_message.GetTranslatedPort(0); - - if (child_message.GetTranslatedPort(1) == MACH_PORT_NULL) { - LOG(ERROR) << "parent GetTranslatedPort(1) failed."; - return pid; - } - MachPortSender parent_sender(child_message.GetTranslatedPort(1)); - - MachSendMessage parent_message(/* id= */0); - if (!parent_message.AddDescriptor(bootstrap_port)) { - LOG(ERROR) << "parent AddDescriptor(" << bootstrap_port << ") failed."; - return pid; - } - - err = parent_sender.SendMessage(parent_message, kTimeoutMs); - if (err != KERN_SUCCESS) { - LOG(ERROR) << "parent SendMessage() failed: " << MachErrorCode(err); - return pid; - } - break; - } - } - return pid; +bool LaunchApp(const std::vector& argv, + const file_handle_mapping_vector& fds_to_remap, + bool wait, ProcessHandle* process_handle) { + return LaunchApp(argv, fds_to_remap, environment_map(), + wait, process_handle); } bool LaunchApp(const std::vector& argv, const file_handle_mapping_vector& fds_to_remap, - bool wait, ProcessHandle* process_handle, - task_t* process_task) { - return LaunchApp(argv, fds_to_remap, environment_map(), - wait, process_handle, process_task); -} + const environment_map& env_vars_to_set, + bool wait, ProcessHandle* process_handle) { + bool retval = true; -bool LaunchApp( - const std::vector& argv, - const file_handle_mapping_vector& fds_to_remap, - const environment_map& environ, - bool wait, - ProcessHandle* process_handle, - task_t* task_handle) { - pid_t pid; + char* argv_copy[argv.size() + 1]; + for (size_t i = 0; i < argv.size(); i++) { + argv_copy[i] = const_cast(argv[i].c_str()); + } + argv_copy[argv.size()] = NULL; - if (task_handle == NULL) { - pid = fork(); - } else { - // On OS X, the task_t for a process is needed for several reasons. Sadly, - // the function task_for_pid() requires privileges a normal user doesn't - // have. Instead, a short-lived Mach IPC connection is opened between parent - // and child, and the child sends its task_t to the parent at fork time. - *task_handle = MACH_PORT_NULL; - pid = fork_and_get_task(task_handle); + // Make sure we don't leak any FDs to the child process by marking all FDs + // as close-on-exec. + SetAllFDsToCloseOnExec(); + + // Copy _NSGetEnviron() to a new char array and add the variables + // in env_vars_to_set. + // Existing variables are overwritten by env_vars_to_set. + int pos = 0; + environment_map combined_env_vars = env_vars_to_set; + while((*_NSGetEnviron())[pos] != NULL) { + std::string varString = (*_NSGetEnviron())[pos]; + std::string varName = varString.substr(0, varString.find_first_of('=')); + std::string varValue = varString.substr(varString.find_first_of('=') + 1); + if (combined_env_vars.find(varName) == combined_env_vars.end()) { + combined_env_vars[varName] = varValue; + } + pos++; + } + int varsLen = combined_env_vars.size() + 1; + + char** vars = new char*[varsLen]; + int i = 0; + for (environment_map::const_iterator it = combined_env_vars.begin(); + it != combined_env_vars.end(); ++it) { + std::string entry(it->first); + entry += "="; + entry += it->second; + vars[i] = strdup(entry.c_str()); + i++; + } + vars[i] = NULL; + + posix_spawn_file_actions_t file_actions; + if (posix_spawn_file_actions_init(&file_actions) != 0) { + for(int j = 0; j < varsLen; j++) { + free(vars[j]); + } + delete[] vars; + return false; } - if (pid < 0) - return false; + // Turn fds_to_remap array into a set of dup2 calls. + for (file_handle_mapping_vector::const_iterator it = fds_to_remap.begin(); + it != fds_to_remap.end(); + ++it) { + int src_fd = it->first; + int dest_fd = it->second; - if (pid == 0) { - // Child process - - 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)); - } - - for (environment_map::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); + if (src_fd == dest_fd) { + int flags = fcntl(src_fd, F_GETFD); + if (flags != -1) { + fcntl(src_fd, F_SETFD, flags & ~FD_CLOEXEC); + } + } else { + if (posix_spawn_file_actions_adddup2(&file_actions, src_fd, dest_fd) != 0) { + posix_spawn_file_actions_destroy(&file_actions); + for(int j = 0; j < varsLen; j++) { + free(vars[j]); + } + delete[] vars; + return false; } } + } - // 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); + int pid = 0; + int spawn_succeeded = (posix_spawnp(&pid, + argv_copy[0], + &file_actions, + NULL, + argv_copy, + vars) == 0); - // 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"); + for(int j = 0; j < varsLen; j++) { + free(vars[j]); + } + delete[] vars; - CloseSuperfluousFds(fd_shuffle); + posix_spawn_file_actions_destroy(&file_actions); - scoped_array argv_cstr(new char*[argv.size() + 1]); - for (size_t i = 0; i < argv.size(); i++) - argv_cstr[i] = const_cast(argv[i].c_str()); - argv_cstr[argv.size()] = NULL; - execvp(argv_cstr[0], argv_cstr.get()); - _exit(127); + bool process_handle_valid = pid > 0; + if (!spawn_succeeded || !process_handle_valid) { + retval = false; } else { - // Parent process if (wait) HANDLE_EINTR(waitpid(pid, 0, 0)); @@ -210,14 +132,14 @@ bool LaunchApp( *process_handle = pid; } - return true; + return retval; } bool LaunchApp(const CommandLine& cl, bool wait, bool start_hidden, ProcessHandle* process_handle) { // TODO(playmobil): Do we need to respect the start_hidden flag? file_handle_mapping_vector no_files; - return LaunchApp(cl.argv(), no_files, wait, process_handle, NULL); + return LaunchApp(cl.argv(), no_files, wait, process_handle); } NamedProcessIterator::NamedProcessIterator(const std::wstring& executable_name, diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index f705d0986db..587dc3f008c 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -43,6 +43,10 @@ #include "base/string_util.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/process_watcher.h" +#ifdef XP_MACOSX +#include "chrome/common/mach_ipc_mac.h" +#include "base/rand_util.h" +#endif #include "prprf.h" @@ -224,9 +228,6 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector aExtraOpts) } base::ProcessHandle process; -#if defined(XP_MACOSX) - task_t child_task; -#endif // send the child the PID so that it can open a ProcessHandle back to us. // probably don't want to do this in the long run @@ -311,7 +312,6 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector aExtraOpts) #endif childArgv.push_back(pidstring); - childArgv.push_back(childProcessType); #if defined(MOZ_CRASHREPORTER) # if defined(OS_LINUX) @@ -333,15 +333,63 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector aExtraOpts) # endif // OS_LINUX #endif +#ifdef XP_MACOSX + // Add a mach port to the command line so the child can communicate its + // 'task_t' back to the parent. + // + // Put a random number into the channel name, so that a compromised renderer + // can't pretend being the child that's forked off. + std::string mach_connection_name = StringPrintf("org.mozilla.machname.%d", + base::RandInt(0, std::numeric_limits::max())); + childArgv.push_back(mach_connection_name.c_str()); +#endif + + childArgv.push_back(childProcessType); + base::LaunchApp(childArgv, mFileMap, #if defined(OS_LINUX) || defined(OS_MACOSX) newEnvVars, #endif - false, &process -#if defined(XP_MACOSX) - , &child_task + false, &process); + +#ifdef XP_MACOSX + // Wait for the child process to send us its 'task_t' data. + const int kTimeoutMs = 1000; + + MachReceiveMessage child_message; + ReceivePort parent_recv_port(mach_connection_name.c_str()); + kern_return_t err = parent_recv_port.WaitForMessage(&child_message, kTimeoutMs); + if (err != KERN_SUCCESS) { + std::string errString = StringPrintf("0x%x %s", err, mach_error_string(err)); + LOG(ERROR) << "parent WaitForMessage() failed: " << errString; + return false; + } + + task_t child_task = child_message.GetTranslatedPort(0); + if (child_task == MACH_PORT_NULL) { + LOG(ERROR) << "parent GetTranslatedPort(0) failed."; + return false; + } + + if (child_message.GetTranslatedPort(1) == MACH_PORT_NULL) { + LOG(ERROR) << "parent GetTranslatedPort(1) failed."; + return false; + } + MachPortSender parent_sender(child_message.GetTranslatedPort(1)); + + MachSendMessage parent_message(/* id= */0); + if (!parent_message.AddDescriptor(bootstrap_port)) { + LOG(ERROR) << "parent AddDescriptor(" << bootstrap_port << ") failed."; + return false; + } + + err = parent_sender.SendMessage(parent_message, kTimeoutMs); + if (err != KERN_SUCCESS) { + std::string errString = StringPrintf("0x%x %s", err, mach_error_string(err)); + LOG(ERROR) << "parent SendMessage() failed: " << errString; + return false; + } #endif - ); //-------------------------------------------------- #elif defined(OS_WIN) @@ -375,12 +423,14 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector aExtraOpts) #endif cmdLine.AppendLooseValue(UTF8ToWide(pidstring)); - cmdLine.AppendLooseValue(UTF8ToWide(childProcessType)); + #if defined(MOZ_CRASHREPORTER) cmdLine.AppendLooseValue( UTF8ToWide(CrashReporter::GetChildNotificationPipe())); #endif + cmdLine.AppendLooseValue(UTF8ToWide(childProcessType)); + base::LaunchApp(cmdLine, false, false, &process); #else diff --git a/toolkit/xre/nsEmbedFunctions.cpp b/toolkit/xre/nsEmbedFunctions.cpp index 32ba8403c02..a4073903bb8 100644 --- a/toolkit/xre/nsEmbedFunctions.cpp +++ b/toolkit/xre/nsEmbedFunctions.cpp @@ -75,6 +75,9 @@ #include "mozilla/Omnijar.h" #ifdef MOZ_IPC +#if defined(XP_MACOSX) +#include "chrome/common/mach_ipc_mac.h" +#endif #include "nsX11ErrorHandler.h" #include "base/at_exit.h" #include "base/command_line.h" @@ -309,6 +312,77 @@ XRE_InitChildProcess(int aArgc, sChildProcessType = aProcess; + // Complete 'task_t' exchange for Mac OS X. This structure has the same size + // regardless of architecture so we don't have any cross-arch issues here. +#ifdef XP_MACOSX + if (aArgc < 1) + return 1; + const char* const mach_port_name = aArgv[--aArgc]; + + const int kTimeoutMs = 1000; + + MachSendMessage child_message(0); + if (!child_message.AddDescriptor(mach_task_self())) { + NS_WARNING("child AddDescriptor(mach_task_self()) failed."); + return 1; + } + + ReceivePort child_recv_port; + mach_port_t raw_child_recv_port = child_recv_port.GetPort(); + if (!child_message.AddDescriptor(raw_child_recv_port)) { + NS_WARNING("Adding descriptor to message failed"); + return 1; + } + + MachPortSender child_sender(mach_port_name); + kern_return_t err = child_sender.SendMessage(child_message, kTimeoutMs); + if (err != KERN_SUCCESS) { + NS_WARNING("child SendMessage() failed"); + return 1; + } + + MachReceiveMessage parent_message; + err = child_recv_port.WaitForMessage(&parent_message, kTimeoutMs); + if (err != KERN_SUCCESS) { + NS_WARNING("child WaitForMessage() failed"); + return 1; + } + + if (parent_message.GetTranslatedPort(0) == MACH_PORT_NULL) { + NS_WARNING("child GetTranslatedPort(0) failed"); + return 1; + } + err = task_set_bootstrap_port(mach_task_self(), + parent_message.GetTranslatedPort(0)); + if (err != KERN_SUCCESS) { + NS_WARNING("child task_set_bootstrap_port() failed"); + return 1; + } +#endif + +#if defined(MOZ_CRASHREPORTER) + if (aArgc < 1) + return 1; + const char* const crashReporterArg = aArgv[--aArgc]; + +# if defined(XP_WIN) || defined(XP_MACOSX) + // on windows and mac, |crashReporterArg| is the named pipe on which the + // server is listening for requests, or "-" if crash reporting is + // disabled. + if (0 != strcmp("-", crashReporterArg) + && !XRE_SetRemoteExceptionHandler(crashReporterArg)) + return 1; +# elif defined(OS_LINUX) + // on POSIX, |crashReporterArg| is "true" if crash reporting is + // enabled, false otherwise + if (0 != strcmp("false", crashReporterArg) + && !XRE_SetRemoteExceptionHandler(NULL)) + return 1; +# else +# error "OOP crash reporting unsupported on this platform" +# endif +#endif // if defined(MOZ_CRASHREPORTER) + gArgv = aArgv; gArgc = aArgc;