diff --git a/src/main/host/managed_thread.c b/src/main/host/managed_thread.c index b3ab467ecc..2aa6d2cba2 100644 --- a/src/main/host/managed_thread.c +++ b/src/main/host/managed_thread.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -133,8 +134,10 @@ static pid_t _managedthread_fork_exec(ManagedThread* thread, const char* file, const char* const* argv_in, const char* const* envp_in, const char* workingDir, int straceFd, int shimlogFd) { utility_debugAssert(file != NULL); + // We actually use `posix_spawn` here, which is a safer abstraction around + // vfork + exec. - // execve technically takes arrays of pointers to *mutable* char. + // posix_spawn technically takes arrays of pointers to *mutable* char. // conservatively dup here. gchar** argv = g_strdupv((gchar**)argv_in); gchar** envp = g_strdupv((gchar**)envp_in); @@ -146,55 +149,44 @@ static pid_t _managedthread_fork_exec(ManagedThread* thread, const char* file, utility_panic("pipe2: %s", g_strerror(errno)); } - // vfork has superior performance to fork with large workloads. - pid_t pid = vfork(); - - // Beware! Unless you really know what you're doing, don't add any code - // between here and the execvpe below. The forked child process is sharing - // memory and control structures with the parent at this point. See - // `man 2 vfork`. - - switch (pid) { - case -1: - utility_panic("fork failed"); - return -1; - break; - case 0: { - // child - - // *Don't* close the write end of the pipe on exec. - if (fcntl(pipefd[1], F_SETFD, 0)) { - die_after_vfork(); - } + posix_spawn_file_actions_t file_actions; + if (posix_spawn_file_actions_init(&file_actions) != 0) { + utility_panic("posix_spawn_file_actions_init: %s", g_strerror(errno)); + } - // clear the FD_CLOEXEC flag for the strace fd so that it's available in the shim - if (straceFd >= 0 && fcntl(straceFd, F_SETFD, 0)) { - die_after_vfork(); - } + // Dup the write end of the pipe; the dup'd descriptor won't have O_CLOEXEC set. + if (posix_spawn_file_actions_adddup2(&file_actions, pipefd[1], pipefd[1]) != 0) { + utility_panic("posix_spawn_file_actions_adddup2: %s", g_strerror(errno)); + } - // set stdout/stderr as the shim log, and clear the FD_CLOEXEC flag so that it's - // available in the shim - if (dup2(shimlogFd, STDOUT_FILENO) < 0) { - die_after_vfork(); - } - if (dup2(shimlogFd, STDERR_FILENO) < 0) { - die_after_vfork(); - } + // Likewise for straceFd. + if (straceFd >= 0) { + if (posix_spawn_file_actions_adddup2(&file_actions, straceFd, straceFd) != 0) { + utility_panic("posix_spawn_file_actions_adddup2: %s", g_strerror(errno)); + } + } - // Set the working directory - if (chdir(workingDir) < 0) { - die_after_vfork(); - } + // set stdout/stderr as the shim log, and clear the FD_CLOEXEC flag so that it's + // available in the shim + if (posix_spawn_file_actions_adddup2(&file_actions, shimlogFd, STDOUT_FILENO) != 0) { + utility_panic("posix_spawn_file_actions_adddup2: %s", g_strerror(errno)); + } + if (posix_spawn_file_actions_adddup2(&file_actions, shimlogFd, STDERR_FILENO) != 0) { + utility_panic("posix_spawn_file_actions_adddup2: %s", g_strerror(errno)); + } - int rc = execvpe(file, argv, envp); - if (rc == -1) { - die_after_vfork(); - } - // Unreachable - die_after_vfork(); - } - default: // parent - break; + // Set the working directory + if (posix_spawn_file_actions_addchdir_np(&file_actions, workingDir) != 0) { + utility_panic("posix_spawn_file_actions_addchdir_np: %s", g_strerror(errno)); + } + + pid_t pid; + if (posix_spawn(&pid, file, &file_actions, NULL, argv, envp) != 0) { + utility_panic("posix_spawn: %s", g_strerror(errno)); + } + + if (posix_spawn_file_actions_destroy(&file_actions) != 0) { + utility_panic("posix_spawn_file_actions: %s", g_strerror(errno)); } // *Must* close the write-end of the pipe, so that the child's copy is the