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 +}