From 171ceb4a00294c700c0ba6906a3a3abad846699e Mon Sep 17 00:00:00 2001 From: Chris Down Date: Tue, 11 Nov 2025 04:26:10 +0800 Subject: [PATCH] core: Verify inherited FDs are writable for stdout/stderr When inheriting file descriptors for stdout/stderr (either from stdin or when making stderr inherit from stdout), we previously just assumed they would be writable and dup'd them. This could lead to broken setups if the inherited FD was actually opened read-only. Before dup'ing any inherited FDs to stdout/stderr, verify they are actually writable using the new fd_is_writable() helper. If not, fall back to /dev/null (or reopen the terminal in the TTY case) with a warning, rather than silently creating a broken setup where output operations would fail. --- src/core/exec-invoke.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index e9c4e3f624f..7943fdf8b7f 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); -- 2.47.3