From dc19c0d7c4ceef065cdc26ed7067af89badfde68 Mon Sep 17 00:00:00 2001 From: Bernard Ladenthin Date: Sun, 16 Aug 2026 13:24:28 +0200 Subject: [PATCH] Keep pipe ends off the standard descriptors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #115 creates the stdio pipes close-on-exec, and subprocess_create_ex then installs them on 0, 1 and 2. Duplicating a descriptor onto itself is a no-op, so an end that already landed on a standard descriptor keeps FD_CLOEXEC and exec closes the child's stream. It happens whenever the parent runs with one of those descriptors free. posix_spawn_file_actions_adddup2 is documented on macOS as behaving "as if dup2() had been called", with no special case for equal descriptors, and that is where it bites. glibc applies the POSIX clarification and clears the flag, which is why no Linux runner shows this. subprocess_fds_above_std moves such an end out of the way at creation time. That removes the self-duplication for every caller rather than patching each site, and it needs no platform test. The added test checks the precondition rather than the symptom: no pipe end may sit on 0, 1 or 2. That is observable everywhere, and it goes red on unpatched main — the symptom itself cannot be reproduced off macOS. Measured: full suite green on x86_64 glibc, and green on macOS in a run that combined this with the AIX branch and the pending utest.h changes, where the same suite had been failing on exactly this before. --- subprocess.h | 47 +++++++++++++++++++++++++++++++++++++++++++++-- test/test.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/subprocess.h b/subprocess.h index b6f9534..c28a408 100644 --- a/subprocess.h +++ b/subprocess.h @@ -682,6 +682,49 @@ int subprocess_create_named_pipe_helper(void **rd, void **wr) { #endif #if !defined(_WIN32) +/* Move a pipe end off 0, 1 or 2. Duplicating a descriptor onto itself is a + no-op, so a pipe end already sitting on a standard descriptor would keep its + FD_CLOEXEC and be closed by exec, leaving the child without that stream. */ +static int subprocess_fds_above_std(int fds[2]) { + int fd_flags; + int index; + int moved; + int saved_errno; + + for (index = 0; index < 2; index++) { + if (fds[index] > STDERR_FILENO) { + continue; + } + + moved = fcntl(fds[index], F_DUPFD, STDERR_FILENO + 1); + if (-1 != moved) { + fd_flags = fcntl(moved, F_GETFD, 0); + if ((-1 == fd_flags) || + (-1 == fcntl(moved, F_SETFD, fd_flags | FD_CLOEXEC))) { + saved_errno = errno; + close(moved); + errno = saved_errno; + moved = -1; + } + } + + if (-1 == moved) { + saved_errno = errno; + close(fds[0]); + close(fds[1]); + fds[0] = -1; + fds[1] = -1; + errno = saved_errno; + return -1; + } + + close(fds[index]); + fds[index] = moved; + } + + return 0; +} + /* Create pipes with close-on-exec set so later subprocesses do not inherit descriptors belonging to subprocesses which are already running. */ static int subprocess_pipe_cloexec(int fds[2]) { @@ -693,7 +736,7 @@ static int subprocess_pipe_cloexec(int fds[2]) { defined(__OpenBSD__) || defined(__DragonFly__) || \ (defined(__sun) && defined(__SVR4)) if (0 == pipe2(fds, O_CLOEXEC)) { - return 0; + return subprocess_fds_above_std(fds); } /* Older kernels can lack pipe2 even when the C library declares it. */ @@ -720,7 +763,7 @@ static int subprocess_pipe_cloexec(int fds[2]) { } } - return 0; + return subprocess_fds_above_std(fds); } #endif diff --git a/test/test.c b/test/test.c index 82910d3..1b2f0f3 100644 --- a/test/test.c +++ b/test/test.c @@ -70,3 +70,55 @@ UTEST(c, create_does_not_inherit_unlisted_windows_handle) { "subprocess inherited a handle outside its standard streams"); } #endif + +UTEST(c, create_keeps_pipe_ends_off_the_standard_descriptors) { +#if defined(_WIN32) + UTEST_SKIP("POSIX file-descriptor test"); +#else + const char *const command_line[] = {"./process_return_zero", 0}; + struct subprocess_s process; + int saved[3]; + int index; + int created; + int stdin_fd = -1; + int stdout_fd = -1; + int restored = 1; + + /* With 0, 1 and 2 all free the pipes land on them, and a later dup2 onto the + same descriptor is a no-op that leaves FD_CLOEXEC set. */ + for (index = 0; index < 3; index++) { + saved[index] = dup(index); + ASSERT_TRUE(0 <= saved[index]); + } + for (index = 0; index < 3; index++) { + close(index); + } + + /* Nothing may assert or print until the standard descriptors are back. */ + created = subprocess_create(command_line, 0, &process); + if (0 == created) { + if (subprocess_stdin(&process)) { + stdin_fd = fileno(subprocess_stdin(&process)); + } + if (subprocess_stdout(&process)) { + stdout_fd = fileno(subprocess_stdout(&process)); + } + subprocess_join(&process, 0); + subprocess_destroy(&process); + } + + for (index = 0; index < 3; index++) { + if (-1 == dup2(saved[index], index)) { + restored = 0; + } + close(saved[index]); + } + + ASSERT_TRUE(restored); + ASSERT_EQ(0, created); + EXPECT_GT_MSG(stdin_fd, STDERR_FILENO, + "a pipe end sits on a standard descriptor"); + EXPECT_GT_MSG(stdout_fd, STDERR_FILENO, + "a pipe end sits on a standard descriptor"); +#endif +}