diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index e0dcbcecd1..834b162c47 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -998,6 +998,21 @@ int fd_vet_accmode(int fd, int mode) { return -EPROTOTYPE; } +int fd_is_writable(int fd) { + int r; + + assert(fd >= 0); + + r = fd_vet_accmode(fd, O_WRONLY); + if (r >= 0) + return true; + + if (IN_SET(r, -EPROTOTYPE, -EBADFD, -EISDIR)) + return false; + + return r; +} + int fd_verify_safe_flags_full(int fd, int extra_flags) { int flags, unexpected_flags; diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index baa81b6a66..8974188194 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -152,6 +152,7 @@ int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd); int fd_is_opath(int fd); int fd_vet_accmode(int fd, int mode); +int fd_is_writable(int fd); int fd_verify_safe_flags_full(int fd, int extra_flags); static inline int fd_verify_safe_flags(int fd) { diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index e9c4e3f624..7943fdf8b7 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -507,9 +507,6 @@ static int setup_output( i = fixup_input(context, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); o = fixup_output(context->std_output, socket_fd); - // FIXME: we probably should spend some time here to verify that if we inherit an fd from stdin - // (possibly indirect via inheritance from stdout) it is actually opened for write! - if (fileno == STDERR_FILENO) { ExecOutput e; e = fixup_output(context->std_error, socket_fd); @@ -526,8 +523,17 @@ static int setup_output( return fileno; /* Duplicate from stdout if possible */ - if (can_inherit_stderr_from_stdout(context, o, e)) + if (can_inherit_stderr_from_stdout(context, o, e)) { + r = fd_is_writable(STDOUT_FILENO); + if (r <= 0) { + if (r < 0) + log_warning_errno(r, "Failed to check if inherited stdout is writable for stderr, falling back to /dev/null."); + else + log_warning("Inherited stdout is not writable for stderr, falling back to /dev/null."); + return open_null_as(O_WRONLY, fileno); + } return RET_NERRNO(dup2(STDOUT_FILENO, fileno)); + } o = e; @@ -537,8 +543,19 @@ static int setup_output( return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); /* If the input is connected to anything that's not a /dev/null or a data fd, inherit that... */ - if (!IN_SET(i, EXEC_INPUT_NULL, EXEC_INPUT_DATA)) + if (!IN_SET(i, EXEC_INPUT_NULL, EXEC_INPUT_DATA)) { + r = fd_is_writable(STDIN_FILENO); + if (r <= 0) { + if (r < 0) + log_warning_errno(r, "Failed to check if inherited stdin is writable for %s, falling back to /dev/null.", + fileno == STDOUT_FILENO ? "stdout" : "stderr"); + else + log_warning("Inherited stdin is not writable for %s, falling back to /dev/null.", + fileno == STDOUT_FILENO ? "stdout" : "stderr"); + return open_null_as(O_WRONLY, fileno); + } return RET_NERRNO(dup2(STDIN_FILENO, fileno)); + } /* If we are not started from PID 1 we just inherit STDOUT from our parent process. */ if (getppid() != 1) @@ -554,8 +571,19 @@ static int setup_output( return open_null_as(O_WRONLY, fileno); case EXEC_OUTPUT_TTY: - if (exec_input_is_terminal(i)) + if (exec_input_is_terminal(i)) { + r = fd_is_writable(STDIN_FILENO); + if (r <= 0) { + if (r < 0) + log_warning_errno(r, "Failed to check if inherited stdin is writable for TTY's %s, falling back to opening terminal.", + fileno == STDOUT_FILENO ? "stdout" : "stderr"); + else + log_warning("Inherited stdin is not writable for TTY's %s, falling back to opening terminal.", + fileno == STDOUT_FILENO ? "stdout" : "stderr"); + return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); + } return RET_NERRNO(dup2(STDIN_FILENO, fileno)); + } return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index a22c0e3119..db869b5d11 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -903,4 +903,27 @@ TEST(fd_vet_accmode) { ASSERT_ERROR(fd_vet_accmode(fd_opath, O_RDWR), EBADFD); } +TEST(fd_is_writable) { + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-fd-writable.XXXXXX"; + _cleanup_close_ int fd_ro = -EBADF, fd_wo = -EBADF, fd_rw = -EBADF, fd_path = -EBADF; + + ASSERT_OK(fd_rw = mkostemp_safe(name)); + ASSERT_OK_POSITIVE(fd_is_writable(fd_rw)); + + ASSERT_OK(fd_ro = open(name, O_RDONLY | O_CLOEXEC)); + ASSERT_OK_ZERO(fd_is_writable(fd_ro)); + + ASSERT_OK(fd_wo = open(name, O_WRONLY | O_CLOEXEC)); + ASSERT_OK_POSITIVE(fd_is_writable(fd_wo)); + + ASSERT_OK(fd_path = open(name, O_PATH | O_CLOEXEC)); + ASSERT_OK_ZERO(fd_is_writable(fd_path)); + + ASSERT_SIGNAL(fd_is_writable(-1), SIGABRT); + + safe_close(fd_ro); + ASSERT_ERROR(fd_is_writable(fd_ro), EBADF); + TAKE_FD(fd_ro); +} + DEFINE_TEST_MAIN(LOG_DEBUG);