diff --git a/src/mp/util.cpp b/src/mp/util.cpp index 42cda9ce..f524ab23 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -60,6 +60,18 @@ size_t MaxFd() } } +//! Report an error and exit from the post-fork child of a multi-threaded +//! process, where only async-signal-safe calls (like write and _exit) are +//! allowed. Accepting only a reference to a char array (in practice a string +//! literal) ensures no allocation is needed at the call site. +template +[[noreturn]] void ChildFail(const char (&msg)[N]) noexcept +{ + const ssize_t written = ::write(STDERR_FILENO, msg, N - 1); + (void)written; + _exit(126); +} + } // namespace std::string ThreadName(const char* exe_name) @@ -130,11 +142,6 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ const std::vector args{connect_info_to_args(std::to_string(fds[0]))}; const std::vector argv{MakeArgv(args)}; - // Clear FD_CLOEXEC on fds[0] before forking so it survives exec in the child. - int fds0_flags; - KJ_SYSCALL(fds0_flags = fcntl(fds[0], F_GETFD)); - KJ_SYSCALL(fcntl(fds[0], F_SETFD, fds0_flags & ~FD_CLOEXEC)); - ProcessId pid = fork(); if (pid == -1) { throw std::system_error(errno, std::system_category(), "fork"); @@ -147,10 +154,7 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ (void)close(fds[1]); throw std::system_error(errno, std::system_category(), "close"); } - static constexpr char msg[] = "SpawnProcess(child): close(fds[1]) failed\n"; - const ssize_t writeResult = ::write(STDERR_FILENO, msg, sizeof(msg) - 1); - (void)writeResult; - _exit(126); + ChildFail("SpawnProcess(child): close(fds[1]) failed\n"); } if (!pid) { @@ -163,6 +167,16 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ } } + // Clear FD_CLOEXEC on socket 0 so it survives exec. Doing this here + // rather than in the parent before forking avoids a window where a + // concurrent fork in another thread would leak the descriptor into an + // unrelated child process (which would not clear it). + // fcntl is async-signal-safe. + const int fds0_flags = fcntl(fds[0], F_GETFD); + if (fds0_flags == -1 || fcntl(fds[0], F_SETFD, fds0_flags & ~FD_CLOEXEC) == -1) { + ChildFail("SpawnProcess(child): clearing FD_CLOEXEC failed\n"); + } + execvp(argv[0], argv.data()); // NOTE: perror() is not async-signal-safe; calling it here in a // post-fork child may deadlock in multithreaded parents.