diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index 555ca90..f2e884d 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -116,14 +116,24 @@ extension Execution { // pidfd_send_signal does not support sending signal to process group try _kill(pid, signal: signal) } else { - guard _pidfd_send_signal( + // Try pidfd_send_signal first + if _pidfd_send_signal( processIdentifier.processDescriptor, signal.rawValue - ) == 0 else { - throw SubprocessError( - code: .init(.failedToSendSignal(signal.rawValue)), - underlyingError: .init(rawValue: errno) - ) + ) == 0 { + return + } else { + if errno == ENOSYS { + // pidfd_send_signal is not implemented in this system + // fallback to _kill() + try _kill(pid, signal: signal) + } else { + // Throw all other errors + throw SubprocessError( + code: .init(.failedToSendSignal(signal.rawValue)), + underlyingError: .init(rawValue: errno) + ) + } } } #else diff --git a/Sources/_SubprocessCShims/process_shims.c b/Sources/_SubprocessCShims/process_shims.c index 906f70a..c10d7c1 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/Sources/_SubprocessCShims/process_shims.c @@ -34,8 +34,15 @@ #include #include #include - +#include #include +#include + +#if __has_include() +#include +#endif + +#endif // TARGET_OS_WINDOWS #if __has_include() #include @@ -50,6 +57,7 @@ extern char **environ; #include #endif +#if !TARGET_OS_WINDOWS int _was_process_exited(int status) { return WIFEXITED(status); } @@ -70,24 +78,8 @@ int _was_process_suspended(int status) { return WIFSTOPPED(status); } -#if TARGET_OS_LINUX -#include - -int _shims_snprintf( - char * _Nonnull str, - int len, - const char * _Nonnull format, - char * _Nonnull str1, - char * _Nonnull str2 -) { - return snprintf(str, len, format, str1, str2); -} - -int _pidfd_send_signal(int pidfd, int signal) { - return syscall(SYS_pidfd_send_signal, pidfd, signal, NULL, 0); -} +#endif // !TARGET_OS_WINDOWS -#endif #if __has_include() vm_size_t _subprocess_vm_size(void) { @@ -96,40 +88,6 @@ vm_size_t _subprocess_vm_size(void) { } #endif -// MARK: - Private Helpers -static pthread_mutex_t _subprocess_fork_lock = PTHREAD_MUTEX_INITIALIZER; - -static int _subprocess_block_everything_but_something_went_seriously_wrong_signals(sigset_t *old_mask) { - sigset_t mask; - int r = 0; - r |= sigfillset(&mask); - r |= sigdelset(&mask, SIGABRT); - r |= sigdelset(&mask, SIGBUS); - r |= sigdelset(&mask, SIGFPE); - r |= sigdelset(&mask, SIGILL); - r |= sigdelset(&mask, SIGKILL); - r |= sigdelset(&mask, SIGSEGV); - r |= sigdelset(&mask, SIGSTOP); - r |= sigdelset(&mask, SIGSYS); - r |= sigdelset(&mask, SIGTRAP); - - r |= pthread_sigmask(SIG_BLOCK, &mask, old_mask); - return r; -} - -#define _subprocess_precondition(__cond) do { \ - int eval = (__cond); \ - if (!eval) { \ - __builtin_trap(); \ - } \ -} while(0) - -#if __DARWIN_NSIG -# define _SUBPROCESS_SIG_MAX __DARWIN_NSIG -#else -# define _SUBPROCESS_SIG_MAX 32 -#endif - // MARK: - Darwin (posix_spawn) #if TARGET_OS_MAC @@ -315,6 +273,16 @@ int _pidfd_open(pid_t pid) { return syscall(SYS_pidfd_open, pid, 0); } +// SYS_pidfd_send_signal is only defined on Linux Kernel 5.1 and above +// Define our dummy value if it's not available +#ifndef SYS_pidfd_send_signal +#define SYS_pidfd_send_signal 424 +#endif + +int _pidfd_send_signal(int pidfd, int signal) { + return syscall(SYS_pidfd_send_signal, pidfd, signal, NULL, 0); +} + // SYS_clone3 is only defined on Linux Kernel 5.3 and above // Define our dummy value if it's not available (as is the case with Musl libc) #ifndef SYS_clone3 @@ -351,6 +319,140 @@ static int _clone3(int *pidfd) { return syscall(SYS_clone3, &args, sizeof(args)); } +struct linux_dirent64 { + unsigned long d_ino; + unsigned long d_off; + unsigned short d_reclen; + unsigned char d_type; + char d_name[]; +}; + +static int _getdents64(int fd, struct linux_dirent64 *dirp, size_t nbytes) { + return syscall(SYS_getdents64, fd, dirp, nbytes); +} + +static pthread_mutex_t _subprocess_fork_lock = PTHREAD_MUTEX_INITIALIZER; + +static int _subprocess_make_critical_mask(sigset_t *old_mask) { + sigset_t mask; + int r = 0; + r |= sigfillset(&mask); + r |= sigdelset(&mask, SIGABRT); + r |= sigdelset(&mask, SIGBUS); + r |= sigdelset(&mask, SIGFPE); + r |= sigdelset(&mask, SIGILL); + r |= sigdelset(&mask, SIGKILL); + r |= sigdelset(&mask, SIGSEGV); + r |= sigdelset(&mask, SIGSTOP); + r |= sigdelset(&mask, SIGSYS); + r |= sigdelset(&mask, SIGTRAP); + + r |= pthread_sigmask(SIG_BLOCK, &mask, old_mask); + return r; +} + +#define _subprocess_precondition(__cond) do { \ + int eval = (__cond); \ + if (!eval) { \ + __builtin_trap(); \ + } \ +} while(0) + +#if __DARWIN_NSIG /* Darwin */ +# define _SUBPROCESS_SIG_MAX __DARWIN_NSIG +#elif defined(NSIG_MAX) /* POSIX issue 8 */ +# define _SUBPROCESS_SIG_MAX NSIG_MAX +#elif defined(_SIG_MAXSIG) /* FreeBSD */ +# define _SUBPROCESS_SIG_MAX _SIG_MAXSIG +#elif defined(_SIGMAX) /* QNX */ +# define _SUBPROCESS_SIG_MAX (_SIGMAX + 1) +#elif defined(NSIG) /* 99% of everything else */ +# define _SUBPROCESS_SIG_MAX NSIG +#else /* Last resort */ +# define _SUBPROCESS_SIG_MAX (sizeof(sigset_t) * CHAR_BIT + 1) +#endif + +int _shims_snprintf( + char * _Nonnull str, + int len, + const char * _Nonnull format, + char * _Nonnull str1, + char * _Nonnull str2 +) { + return snprintf(str, len, format, str1, str2); +} + +static int _positive_int_parse(const char *str) { + char *end; + long value = strtol(str, &end, 10); + if (end == str) { + // No digits found + return -1; + } + if (errno == ERANGE || value <= 0 || value > INT_MAX) { + // Out of range + return -1; + } + return (int)value; +} + +// Linux-specific version that uses syscalls directly and doesn't allocate heap memory. +// Safe to use after vfork() and before execve() +static int _highest_possibly_open_fd_dir_linux(const char *fd_dir) { + int highest_fd_so_far = 0; + int dir_fd = open(fd_dir, O_RDONLY); + if (dir_fd < 0) { + // errno set by `open`. + return -1; + } + + // Buffer for directory entries - allocated on stack, no heap allocation + char buffer[4096] = {0}; + long bytes_read = -1; + + while ((bytes_read = _getdents64(dir_fd, (struct linux_dirent64 *)buffer, sizeof(buffer))) > 0) { + if (bytes_read < 0) { + if (errno == EINTR) { + continue; + } else { + // `errno` set by _getdents64. + highest_fd_so_far = -1; + goto error; + } + } + long offset = 0; + while (offset < bytes_read) { + struct linux_dirent64 *entry = (struct linux_dirent64 *)(buffer + offset); + + // Skip "." and ".." entries + if (entry->d_name[0] != '.') { + int number = _positive_int_parse(entry->d_name); + if (number > highest_fd_so_far) { + highest_fd_so_far = number; + } + } + + offset += entry->d_reclen; + } + } + +error: + close(dir_fd); + return highest_fd_so_far; +} + +static int _highest_possibly_open_fd(void) { +#if defined(__linux__) + int hi = _highest_possibly_open_fd_dir_linux("/dev/fd"); + if (hi < 0) { + hi = sysconf(_SC_OPEN_MAX); + } +#else + int hi = sysconf(_SC_OPEN_MAX); +#endif + return hi; +} + int _subprocess_fork_exec( pid_t * _Nonnull pid, int * _Nonnull pidfd, @@ -410,7 +512,7 @@ int _subprocess_fork_exec( _subprocess_precondition(rc == 0); // Block all signals on this thread sigset_t old_sigmask; - rc = _subprocess_block_everything_but_something_went_seriously_wrong_signals(&old_sigmask); + rc = _subprocess_make_critical_mask(&old_sigmask); if (rc != 0) { close(pipefd[0]); close(pipefd[1]); @@ -530,20 +632,22 @@ int _subprocess_fork_exec( if (rc < 0) { write_error_and_exit; } - - // Close parent side - if (file_descriptors[1] >= 0) { - rc = close(file_descriptors[1]); - } - if (file_descriptors[3] >= 0) { - rc = close(file_descriptors[3]); - } - if (file_descriptors[5] >= 0) { - rc = close(file_descriptors[5]); - } - - if (rc < 0) { - write_error_and_exit; + // Close all other file descriptors + rc = -1; + errno = ENOSYS; +#if __has_include() || defined(__FreeBSD__) + // We must NOT close pipefd[1] for writing errors + rc = close_range(STDERR_FILENO + 1, pipefd[1] - 1, 0); + rc |= close_range(pipefd[1] + 1, ~0U, 0); +#endif + if (rc != 0) { + // close_range failed (or doesn't exist), fall back to close() + for (int fd = STDERR_FILENO + 1; fd < _highest_possibly_open_fd(); fd++) { + // We must NOT close pipefd[1] for writing errors + if (fd != pipefd[1]) { + close(fd); + } + } } // Run custom configuratior @@ -621,8 +725,6 @@ int _subprocess_fork_exec( #endif // TARGET_OS_LINUX -#endif // !TARGET_OS_WINDOWS - #pragma mark - Environment Locking #if __has_include() diff --git a/Tests/SubprocessTests/SubprocessTests+Linux.swift b/Tests/SubprocessTests/SubprocessTests+Linux.swift index 0fb6718..2fa25b5 100644 --- a/Tests/SubprocessTests/SubprocessTests+Linux.swift +++ b/Tests/SubprocessTests/SubprocessTests+Linux.swift @@ -64,56 +64,31 @@ struct SubprocessLinuxTests { } @Test func testSuspendResumeProcess() async throws { - let result = try await Subprocess.run( + _ = try await Subprocess.run( // This will intentionally hang .path("/usr/bin/sleep"), arguments: ["infinity"], - output: .discarded, error: .discarded - ) { subprocess -> Error? in - do { - try await tryFinally { - // First suspend the process - try subprocess.send(signal: .suspend) - try await waitForCondition(timeout: .seconds(30), comment: "Process did not transition from running to stopped state after $$") { - let state = try subprocess.state() - switch state { - case .running: - return false - case .zombie: - throw ProcessStateError(expectedState: .stopped, actualState: state) - case .stopped, .sleeping, .uninterruptibleWait: - return true - } - } - // Now resume the process - try subprocess.send(signal: .resume) - try await waitForCondition(timeout: .seconds(30), comment: "Process did not transition from stopped to running state after $$") { - let state = try subprocess.state() - switch state { - case .running, .sleeping, .uninterruptibleWait: - return true - case .zombie: - throw ProcessStateError(expectedState: .running, actualState: state) - case .stopped: - return false - } - } - } finally: { error in - // Now kill the process - try subprocess.send(signal: error != nil ? .kill : .terminate) + ) { subprocess, standardOutput in + try await tryFinally { + // First suspend the process + try subprocess.send(signal: .suspend) + try await waitForCondition(timeout: .seconds(30)) { + let state = try subprocess.state() + return state == .stopped } - return nil - } catch { - return error + // Now resume the process + try subprocess.send(signal: .resume) + try await waitForCondition(timeout: .seconds(30)) { + let state = try subprocess.state() + return state == .running + } + } finally: { error in + // Now kill the process + try subprocess.send(signal: error != nil ? .kill : .terminate) + for try await _ in standardOutput {} } } - if let error = result.value { - #expect(result.terminationStatus == .unhandledException(SIGKILL)) - throw error - } else { - #expect(result.terminationStatus == .unhandledException(SIGTERM)) - } } @Test func testUniqueProcessIdentifier() async throws { @@ -141,15 +116,6 @@ fileprivate enum ProcessState: String { case stopped = "T" } -fileprivate struct ProcessStateError: Error, CustomStringConvertible { - let expectedState: ProcessState - let actualState: ProcessState - - var description: String { - "Process did not transition to \(expectedState) state, but was actually \(actualState)" - } -} - extension Execution { fileprivate func state() throws -> ProcessState { let processStatusFile = "/proc/\(processIdentifier.value)/status" @@ -175,7 +141,7 @@ extension Execution { } } -func waitForCondition(timeout: Duration, comment: Comment, _ evaluateCondition: () throws -> Bool) async throws { +func waitForCondition(timeout: Duration, _ evaluateCondition: () throws -> Bool) async throws { var currentCondition = try evaluateCondition() let deadline = ContinuousClock.now + timeout while ContinuousClock.now < deadline { @@ -186,13 +152,11 @@ func waitForCondition(timeout: Duration, comment: Comment, _ evaluateCondition: currentCondition = try evaluateCondition() } struct TimeoutError: Error, CustomStringConvertible { - let timeout: Duration - let comment: Comment var description: String { - comment.description.replacingOccurrences(of: "$$", with: "\(timeout)") + "Timed out waiting for condition to be true" } } - throw TimeoutError(timeout: timeout, comment: comment) + throw TimeoutError() } func tryFinally(_ work: () async throws -> (), finally: (Error?) async throws -> ()) async throws { diff --git a/Tests/SubprocessTests/SubprocessTests+Unix.swift b/Tests/SubprocessTests/SubprocessTests+Unix.swift index e23a246..bbb678a 100644 --- a/Tests/SubprocessTests/SubprocessTests+Unix.swift +++ b/Tests/SubprocessTests/SubprocessTests+Unix.swift @@ -952,9 +952,64 @@ extension SubprocessUnixTests { } try FileManager.default.removeItem(at: testFilePath) } + + @Test(.requiresBash) func testDoesNotInheritRandomFileDescriptorsByDefault() async throws { + // This tests makes sure POSIX_SPAWN_CLOEXEC_DEFAULT works on all platforms + let pipe = try FileDescriptor.ssp_pipe() + defer { + close(pipe.readEnd.rawValue) + } + let writeFd = pipe.writeEnd.rawValue + let result = try await pipe.writeEnd.closeAfter { + return try await Subprocess.run( + .path("/bin/bash"), + arguments: ["-c", "echo hello from child >&\(writeFd); echo wrote into \(writeFd), echo exit code $?"], + output: .string(limit: .max), + error: .string(limit: .max) + ) + } + + #expect(result.terminationStatus.isSuccess) + #expect( + result.standardOutput?.trimmingCharacters(in: .whitespacesAndNewlines) == + "wrote into \(writeFd), echo exit code 1" + ) + // Depending on the platform, standard error should be something like + // `/bin/bash: 7: Bad file descriptor + #expect(!result.standardError!.isEmpty) + let nonInherited = try await pipe.readEnd.readUntilEOF(upToLength: .max) + // We should have read nothing because the pipe is not inherited + #expect(nonInherited.isEmpty) + } } // MARK: - Utils +extension FileDescriptor { + /// Runs a closure and then closes the FileDescriptor, even if an error occurs. + /// + /// - Parameter body: The closure to run. + /// If the closure throws an error, + /// this method closes the file descriptor before it rethrows that error. + /// + /// - Returns: The value returned by the closure. + /// + /// If `body` throws an error + /// or an error occurs while closing the file descriptor, + /// this method rethrows that error. + public func closeAfter(_ body: () async throws -> R) async throws -> R { + // No underscore helper, since the closure's throw isn't necessarily typed. + let result: R + do { + result = try await body() + } catch { + _ = try? self.close() // Squash close error and throw closure's + throw error + } + try self.close() + return result + } +} + extension SubprocessUnixTests { private func assertID( withArgument argument: String,