From eac386776426027a3e1fca1798be90a714234e6b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 04:27:37 +0100 Subject: [PATCH 1/8] core/exec-invoke: fix stdout and stderr sharing the same named fd Previously such usage would be refused by exec_context_named_iofds() with -ENOENT, however setup_output() carried relevant logic. --- src/core/exec-invoke.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index fe836de6c0..7f49555fbe 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -460,8 +460,10 @@ static bool can_inherit_stderr_from_stdout( if (e != o) return false; + /* Let's not shortcut named fds here, even though we in theory can by comparing fd names, since + * we have the named_iofds array readily available, and the inherit practice would simply be duplicative. */ if (e == EXEC_OUTPUT_NAMED_FD) - return streq_ptr(context->stdio_fdname[STDOUT_FILENO], context->stdio_fdname[STDERR_FILENO]); + return false; if (IN_SET(e, EXEC_OUTPUT_FILE, EXEC_OUTPUT_FILE_APPEND, EXEC_OUTPUT_FILE_TRUNCATE)) return streq_ptr(context->stdio_file[STDOUT_FILENO], context->stdio_file[STDERR_FILENO]); @@ -4794,28 +4796,34 @@ static int exec_context_named_iofds( /* Note that socket fds are always placed at the beginning of the fds array, no need for extra * manipulation. */ - for (size_t i = 0; i < p->n_socket_fds && targets > 0; i++) + for (size_t i = 0; i < p->n_socket_fds && targets > 0; i++) { if (named_iofds[STDIN_FILENO] < 0 && c->std_input == EXEC_INPUT_NAMED_FD && streq(p->fd_names[i], stdio_fdname[STDIN_FILENO])) { named_iofds[STDIN_FILENO] = p->fds[i]; targets--; + continue; + } - } else if (named_iofds[STDOUT_FILENO] < 0 && - c->std_output == EXEC_OUTPUT_NAMED_FD && - streq(p->fd_names[i], stdio_fdname[STDOUT_FILENO])) { + /* Allow stdout and stderr to use the same named fd */ + + if (named_iofds[STDOUT_FILENO] < 0 && + c->std_output == EXEC_OUTPUT_NAMED_FD && + streq(p->fd_names[i], stdio_fdname[STDOUT_FILENO])) { named_iofds[STDOUT_FILENO] = p->fds[i]; targets--; + } - } else if (named_iofds[STDERR_FILENO] < 0 && - c->std_error == EXEC_OUTPUT_NAMED_FD && - streq(p->fd_names[i], stdio_fdname[STDERR_FILENO])) { + if (named_iofds[STDERR_FILENO] < 0 && + c->std_error == EXEC_OUTPUT_NAMED_FD && + streq(p->fd_names[i], stdio_fdname[STDERR_FILENO])) { named_iofds[STDERR_FILENO] = p->fds[i]; targets--; } + } return targets == 0 ? 0 : -ENOENT; } From 27660b470fc2ad5f70d8735dfbb382aabbe3ac0e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 03:55:41 +0100 Subject: [PATCH 2/8] core/dbus-execute: path is required if stdio is specified as file This partially reverts d6ff82d37cae5910a0c62bf830e3cfa70df1e23f. --- src/core/dbus-execute.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index b7d8b4b5ff..8f09cb68bf 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -3338,17 +3338,15 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (!isempty(s)) { - if (!path_is_absolute(s)) - return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute", s); - if (!path_is_normalized(s)) - return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not normalized", s); - } + if (!path_is_absolute(s)) + return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute", s); + if (!path_is_normalized(s)) + return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not normalized", s); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "StandardInputFile")) { - r = free_and_strdup(&c->stdio_file[STDIN_FILENO], empty_to_null(s)); + r = free_and_strdup(&c->stdio_file[STDIN_FILENO], s); if (r < 0) return r; @@ -3356,7 +3354,7 @@ int bus_exec_context_set_transient_property( unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardInput=file:%s", s); } else if (STR_IN_SET(name, "StandardOutputFile", "StandardOutputFileToAppend", "StandardOutputFileToTruncate")) { - r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], empty_to_null(s)); + r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], s); if (r < 0) return r; @@ -3374,7 +3372,7 @@ int bus_exec_context_set_transient_property( } else { assert(STR_IN_SET(name, "StandardErrorFile", "StandardErrorFileToAppend", "StandardErrorFileToTruncate")); - r = free_and_strdup(&c->stdio_file[STDERR_FILENO], empty_to_null(s)); + r = free_and_strdup(&c->stdio_file[STDERR_FILENO], s); if (r < 0) return r; From 4dac3c6d76907e5ca7a79e90f6141577cbcea9a4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 07:10:09 +0100 Subject: [PATCH 3/8] core/execute-serialize: clean up stdio serialization * Do not interleave root_directory_as_fd with stdio fields * Do not use different serialization key for different modes pointing to same path * Escape stdio file paths (as per 9be46b1da8b01c3f47e6c050185f2b45484d6300) --- src/core/execute-serialize.c | 168 +++++++++++------------ test/fuzz/fuzz-execute-serialize/initial | 6 +- 2 files changed, 79 insertions(+), 95 deletions(-) diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c index a634b0308e..a652b9a467 100644 --- a/src/core/execute-serialize.c +++ b/src/core/execute-serialize.c @@ -1539,48 +1539,6 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) { return 0; } -static int serialize_std_out_err(const ExecContext *c, FILE *f, int fileno) { - char *key, *value; - const char *type; - - assert(c); - assert(f); - assert(IN_SET(fileno, STDOUT_FILENO, STDERR_FILENO)); - - type = fileno == STDOUT_FILENO ? "output" : "error"; - - switch (fileno == STDOUT_FILENO ? c->std_output : c->std_error) { - case EXEC_OUTPUT_NAMED_FD: - key = strjoina("exec-context-std-", type, "-fd-name"); - value = c->stdio_fdname[fileno]; - - break; - - case EXEC_OUTPUT_FILE: - key = strjoina("exec-context-std-", type, "-file"); - value = c->stdio_file[fileno]; - - break; - - case EXEC_OUTPUT_FILE_APPEND: - key = strjoina("exec-context-std-", type, "-file-append"); - value = c->stdio_file[fileno]; - - break; - - case EXEC_OUTPUT_FILE_TRUNCATE: - key = strjoina("exec-context-std-", type, "-file-truncate"); - value = c->stdio_file[fileno]; - - break; - - default: - return 0; - } - - return serialize_item(f, key, value); -} - static int exec_context_serialize(const ExecContext *c, FILE *f) { int r; @@ -2004,6 +1962,10 @@ static int exec_context_serialize(const ExecContext *c, FILE *f) { return r; } + r = serialize_bool_elide(f, "exec-context-root-directory-as-fd", c->root_directory_as_fd); + if (r < 0) + return r; + r = serialize_item(f, "exec-context-std-input", exec_input_to_string(c->std_input)); if (r < 0) return r; @@ -2020,36 +1982,60 @@ static int exec_context_serialize(const ExecContext *c, FILE *f) { if (r < 0) return r; - r = serialize_bool_elide(f, "exec-context-root-directory-as-fd", c->root_directory_as_fd); - if (r < 0) - return r; - switch (c->std_input) { + case EXEC_INPUT_NAMED_FD: r = serialize_item(f, "exec-context-std-input-fd-name", c->stdio_fdname[STDIN_FILENO]); - if (r < 0) - return r; break; case EXEC_INPUT_FILE: - r = serialize_item(f, "exec-context-std-input-file", c->stdio_file[STDIN_FILENO]); - if (r < 0) - return r; + r = serialize_item_escaped(f, "exec-context-std-input-file", c->stdio_file[STDIN_FILENO]); + break; + + case EXEC_INPUT_DATA: + r = serialize_item_base64mem(f, "exec-context-std-input-data", c->stdin_data, c->stdin_data_size); break; default: - ; + r = 0; } - - r = serialize_std_out_err(c, f, STDOUT_FILENO); if (r < 0) return r; - r = serialize_std_out_err(c, f, STDERR_FILENO); + switch (c->std_output) { + + case EXEC_OUTPUT_NAMED_FD: + r = serialize_item(f, "exec-context-std-output-fd-name", c->stdio_fdname[STDOUT_FILENO]); + break; + + case EXEC_OUTPUT_FILE: + case EXEC_OUTPUT_FILE_APPEND: + case EXEC_OUTPUT_FILE_TRUNCATE: + r = serialize_item_escaped(f, "exec-context-std-output-file", c->stdio_file[STDOUT_FILENO]); + break; + + default: + r = 0; + } if (r < 0) return r; - r = serialize_item_base64mem(f, "exec-context-stdin-data", c->stdin_data, c->stdin_data_size); + + switch (c->std_error) { + + case EXEC_OUTPUT_NAMED_FD: + r = serialize_item(f, "exec-context-std-error-fd-name", c->stdio_fdname[STDERR_FILENO]); + break; + + case EXEC_OUTPUT_FILE: + case EXEC_OUTPUT_FILE_APPEND: + case EXEC_OUTPUT_FILE_TRUNCATE: + r = serialize_item_escaped(f, "exec-context-std-error-file", c->stdio_file[STDERR_FILENO]); + break; + + default: + r = 0; + } if (r < 0) return r; @@ -3017,6 +3003,11 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { r = deserialize_usec(val, (usec_t *)&c->timer_slack_nsec); if (r < 0) return r; + } else if ((val = startswith(l, "exec-context-root-directory-as-fd="))) { + r = parse_boolean(val); + if (r < 0) + return r; + c->root_directory_as_fd = r; } else if ((val = startswith(l, "exec-context-std-input="))) { c->std_input = exec_input_from_string(val); if (c->std_input < 0) @@ -3034,11 +3025,13 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { if (r < 0) return r; c->stdio_as_fds = r; - } else if ((val = startswith(l, "exec-context-root-directory-as-fd="))) { - r = parse_boolean(val); + } else if ((val = startswith(l, "exec-context-std-input-data="))) { + if (c->stdin_data) + return -EINVAL; /* duplicated */ + + r = unbase64mem(val, &c->stdin_data, &c->stdin_data_size); if (r < 0) return r; - c->root_directory_as_fd = r; } else if ((val = startswith(l, "exec-context-std-input-fd-name="))) { r = free_and_strdup(&c->stdio_fdname[STDIN_FILENO], val); if (r < 0) @@ -3052,40 +3045,35 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { if (r < 0) return r; } else if ((val = startswith(l, "exec-context-std-input-file="))) { - r = free_and_strdup(&c->stdio_file[STDIN_FILENO], val); - if (r < 0) - return r; + ssize_t k; + char *p; + + k = cunescape(val, 0, &p); + if (k < 0) + return k; + + free_and_replace(c->stdio_file[STDIN_FILENO], p); + } else if ((val = startswith(l, "exec-context-std-output-file="))) { - r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], val); - if (r < 0) - return r; - } else if ((val = startswith(l, "exec-context-std-output-file-append="))) { - r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], val); - if (r < 0) - return r; - } else if ((val = startswith(l, "exec-context-std-output-file-truncate="))) { - r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], val); - if (r < 0) - return r; + ssize_t k; + char *p; + + k = cunescape(val, 0, &p); + if (k < 0) + return k; + + free_and_replace(c->stdio_file[STDOUT_FILENO], p); + } else if ((val = startswith(l, "exec-context-std-error-file="))) { - r = free_and_strdup(&c->stdio_file[STDERR_FILENO], val); - if (r < 0) - return r; - } else if ((val = startswith(l, "exec-context-std-error-file-append="))) { - r = free_and_strdup(&c->stdio_file[STDERR_FILENO], val); - if (r < 0) - return r; - } else if ((val = startswith(l, "exec-context-std-error-file-truncate="))) { - r = free_and_strdup(&c->stdio_file[STDERR_FILENO], val); - if (r < 0) - return r; - } else if ((val = startswith(l, "exec-context-stdin-data="))) { - if (c->stdin_data) - return -EINVAL; /* duplicated */ + ssize_t k; + char *p; + + k = cunescape(val, 0, &p); + if (k < 0) + return k; + + free_and_replace(c->stdio_file[STDERR_FILENO], p); - r = unbase64mem(val, &c->stdin_data, &c->stdin_data_size); - if (r < 0) - return r; } else if ((val = startswith(l, "exec-context-tty-path="))) { r = free_and_strdup(&c->tty_path, val); if (r < 0) diff --git a/test/fuzz/fuzz-execute-serialize/initial b/test/fuzz/fuzz-execute-serialize/initial index 4d9103c3ab..786ad89678 100644 --- a/test/fuzz/fuzz-execute-serialize/initial +++ b/test/fuzz/fuzz-execute-serialize/initial @@ -63,14 +63,10 @@ exec-context-stdio-as-fds= exec-context-std-input-fd-name= exec-context-std-output-fd-name= exec-context-std-error-fd-name= +exec-context-std-input-data= exec-context-std-input-file= exec-context-std-output-file= -exec-context-std-output-file-append= -exec-context-std-output-file-truncate= exec-context-std-error-file= -exec-context-std-error-file-append= -exec-context-std-error-file-truncate= -exec-context-stdin-data= exec-context-tty-path= exec-context-tty-reset= exec-context-tty-vhangup= From c8a6c42e5309e00658c4014f4e5acd05eea5f41c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 04:00:12 +0100 Subject: [PATCH 4/8] core/exec-invoke: use path_equal() on for stdio files --- src/core/exec-invoke.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 7f49555fbe..7d1a643b80 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -430,8 +430,8 @@ static int setup_input( assert(context->stdio_file[STDIN_FILENO]); - rw = (context->std_output == EXEC_OUTPUT_FILE && streq_ptr(context->stdio_file[STDIN_FILENO], context->stdio_file[STDOUT_FILENO])) || - (context->std_error == EXEC_OUTPUT_FILE && streq_ptr(context->stdio_file[STDIN_FILENO], context->stdio_file[STDERR_FILENO])); + rw = (context->std_output == EXEC_OUTPUT_FILE && path_equal(context->stdio_file[STDIN_FILENO], context->stdio_file[STDOUT_FILENO])) || + (context->std_error == EXEC_OUTPUT_FILE && path_equal(context->stdio_file[STDIN_FILENO], context->stdio_file[STDERR_FILENO])); fd = acquire_path(context->stdio_file[STDIN_FILENO], rw ? O_RDWR : O_RDONLY, 0666 & ~context->umask); if (fd < 0) @@ -466,7 +466,7 @@ static bool can_inherit_stderr_from_stdout( return false; if (IN_SET(e, EXEC_OUTPUT_FILE, EXEC_OUTPUT_FILE_APPEND, EXEC_OUTPUT_FILE_TRUNCATE)) - return streq_ptr(context->stdio_file[STDOUT_FILENO], context->stdio_file[STDERR_FILENO]); + return path_equal(context->stdio_file[STDOUT_FILENO], context->stdio_file[STDERR_FILENO]); return true; } @@ -632,15 +632,14 @@ static int setup_output( case EXEC_OUTPUT_FILE: case EXEC_OUTPUT_FILE_APPEND: case EXEC_OUTPUT_FILE_TRUNCATE: { - bool rw; int fd, flags; assert(context->stdio_file[fileno]); - rw = context->std_input == EXEC_INPUT_FILE && - streq_ptr(context->stdio_file[fileno], context->stdio_file[STDIN_FILENO]); - - if (rw) + /* stdin points to the same file hence setup_input() opened it as rw already? + * Then just duplicate it. */ + if (context->std_input == EXEC_INPUT_FILE && + path_equal(context->stdio_file[fileno], context->stdio_file[STDIN_FILENO])) return RET_NERRNO(dup2(STDIN_FILENO, fileno)); flags = O_WRONLY; From b34aee04400d063ac6910fb1f287d64ef2828057 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 18:56:41 +0100 Subject: [PATCH 5/8] core/exec-invoke: socket_fd must be set if any of stdio requests it This is checked as one of the very first things in exec_invoke(), hence remove the condition in fixup_input() and the whole fixup_output(). --- src/core/exec-invoke.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 7d1a643b80..4312fad505 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -308,7 +308,6 @@ static int acquire_path(const char *path, int flags, mode_t mode) { static int fixup_input( const ExecContext *context, - int socket_fd, bool apply_tty_stdin) { ExecInput std_input; @@ -320,23 +319,12 @@ static int fixup_input( if (exec_input_is_terminal(std_input) && !apply_tty_stdin) return EXEC_INPUT_NULL; - if (std_input == EXEC_INPUT_SOCKET && socket_fd < 0) - return EXEC_INPUT_NULL; - if (std_input == EXEC_INPUT_DATA && context->stdin_data_size == 0) return EXEC_INPUT_NULL; return std_input; } -static int fixup_output(ExecOutput output, int socket_fd) { - - if (output == EXEC_OUTPUT_SOCKET && socket_fd < 0) - return EXEC_OUTPUT_INHERIT; - - return output; -} - static int setup_input( const ExecContext *context, const ExecParameters *params, @@ -361,7 +349,7 @@ static int setup_input( return STDIN_FILENO; } - i = fixup_input(context, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); + i = fixup_input(context, params->flags & EXEC_APPLY_TTY_STDIN); switch (i) { @@ -445,16 +433,17 @@ static int setup_input( } } -static bool can_inherit_stderr_from_stdout( - const ExecContext *context, - ExecOutput o, - ExecOutput e) { +static bool can_inherit_stderr_from_stdout(const ExecContext *context) { + ExecOutput o, e; assert(context); /* Returns true, if given the specified STDERR and STDOUT output we can directly dup() the stdout fd to the * stderr fd */ + o = context->std_output; + e = context->std_error; + if (e == EXEC_OUTPUT_INHERIT) return true; if (e != o) @@ -508,12 +497,11 @@ static int setup_output( return STDERR_FILENO; } - i = fixup_input(context, socket_fd, params->flags & EXEC_APPLY_TTY_STDIN); - o = fixup_output(context->std_output, socket_fd); + i = fixup_input(context, params->flags & EXEC_APPLY_TTY_STDIN); + o = context->std_output; if (fileno == STDERR_FILENO) { - ExecOutput e; - e = fixup_output(context->std_error, socket_fd); + ExecOutput e = context->std_error; /* This expects the input and output are already set up */ @@ -527,7 +515,7 @@ 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)) { r = fd_is_writable(STDOUT_FILENO); if (r <= 0) { if (r < 0) From e04382384de4d70d2d9178138ea7a68b304397ed Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 07:33:22 +0100 Subject: [PATCH 6/8] core/execute: introduce exec_input_is_inheritable() helper --- src/core/execute.h | 10 ++++++++++ src/core/service.c | 14 +++----------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/core/execute.h b/src/core/execute.h index 42127347cf..92a2c730fd 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -480,6 +480,16 @@ static inline bool exec_context_has_tty(const ExecContext *context) { context->std_error == EXEC_OUTPUT_TTY; } +static inline bool exec_input_is_inheritable(ExecInput i) { + /* We assume these listed inputs refer to bidirectional streams, and hence duplicating them from + * stdin to stdout/stderr makes sense and hence allowing EXEC_OUTPUT_INHERIT makes sense, too. + * Outputs such as regular files or sealed data memfds otoh don't really make sense to be + * duplicated for both input and output at the same time (since they then would cause a feedback + * loop). */ + + return exec_input_is_terminal(i) || IN_SET(i, EXEC_INPUT_SOCKET, EXEC_INPUT_NAMED_FD); +} + int exec_spawn( Unit *unit, ExecCommand *command, diff --git a/src/core/service.c b/src/core/service.c index 9f7ccaaa41..3b594b5c5d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -808,19 +808,11 @@ static void service_fix_stdio(Service *s) { s->exec_context.stdin_data_size > 0) s->exec_context.std_input = EXEC_INPUT_DATA; - if (IN_SET(s->exec_context.std_input, - EXEC_INPUT_TTY, - EXEC_INPUT_TTY_FORCE, - EXEC_INPUT_TTY_FAIL, - EXEC_INPUT_SOCKET, - EXEC_INPUT_NAMED_FD)) + if (exec_input_is_inheritable(s->exec_context.std_input)) return; - /* We assume these listed inputs refer to bidirectional streams, and hence duplicating them from - * stdin to stdout/stderr makes sense and hence leaving EXEC_OUTPUT_INHERIT in place makes sense, - * too. Outputs such as regular files or sealed data memfds otoh don't really make sense to be - * duplicated for both input and output at the same time (since they then would cause a feedback - * loop), hence override EXEC_OUTPUT_INHERIT with the default stderr/stdout setting. */ + /* Override EXEC_OUTPUT_INHERIT with the default stderr/stdout setting if not applicable for + * given stdin mode. */ if (s->exec_context.std_error == EXEC_OUTPUT_INHERIT && s->exec_context.std_output == EXEC_OUTPUT_INHERIT) From a8e1796b09f8fa634b1ef2624836bfa326b2ca95 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 22 Nov 2025 19:23:53 +0100 Subject: [PATCH 7/8] core/exec-invoke: split out maybe_inherit_stdout_from_stdin(), use exec_input_is_inheritable() Note that exec_input_is_inheritable() rightfully refuses EXEC_INPUT_FILE, in which case std_output would have been reset in service_fix_stdio() already. While at it, use the generic fallback logic of first trying user manager stdout when stdin is not writable. --- src/core/exec-invoke.c | 77 ++++++++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 4312fad505..7cc377a253 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -460,6 +460,41 @@ static bool can_inherit_stderr_from_stdout(const ExecContext *context) { return true; } +static int maybe_inherit_stdout_from_stdin(const ExecContext *context, ExecInput i) { + int r; + + assert(context); + + if (context->std_output != EXEC_OUTPUT_INHERIT) + return 0; + + /* If input got downgraded, inherit the original value */ + if (i == EXEC_INPUT_NULL && exec_input_is_terminal(context->std_input)) + return open_terminal_as(exec_context_tty_path(context), O_WRONLY, STDOUT_FILENO); + + if (!exec_input_is_inheritable(i)) + goto fallback; + + 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 stdout, using fallback: %m"); + else + log_warning("Inherited stdin is not writable for stdout, using fallback: %m"); + goto fallback; + } + + return RET_NERRNO(dup2(STDIN_FILENO, STDOUT_FILENO)); + +fallback: + /* If we are not started from PID 1 we just inherit STDOUT from our parent process. */ + if (getppid() != 1) + return STDOUT_FILENO; + + /* We need to open /dev/null here anew, to get the right access mode. */ + return open_null_as(O_WRONLY, STDOUT_FILENO); +} + static int setup_output( const ExecContext *context, const ExecParameters *params, @@ -498,19 +533,15 @@ static int setup_output( } i = fixup_input(context, params->flags & EXEC_APPLY_TTY_STDIN); - o = context->std_output; if (fileno == STDERR_FILENO) { - ExecOutput e = context->std_error; - /* This expects the input and output are already set up */ /* Don't change the stderr file descriptor if we inherit all * the way and are not on a tty */ - if (e == EXEC_OUTPUT_INHERIT && - o == EXEC_OUTPUT_INHERIT && - i == EXEC_INPUT_NULL && - !exec_input_is_terminal(context->std_input) && + if (context->std_error == EXEC_OUTPUT_INHERIT && + context->std_output == EXEC_OUTPUT_INHERIT && + i == EXEC_INPUT_NULL && !exec_input_is_terminal(context->std_input) && getppid() != 1) return fileno; @@ -527,34 +558,16 @@ static int setup_output( return RET_NERRNO(dup2(STDOUT_FILENO, fileno)); } - o = e; + o = context->std_error; - } else if (o == EXEC_OUTPUT_INHERIT) { - /* If input got downgraded, inherit the original value */ - if (i == EXEC_INPUT_NULL && exec_input_is_terminal(context->std_input)) - return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno); + } else { + assert(fileno == STDOUT_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)) { - 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)); - } + r = maybe_inherit_stdout_from_stdin(context, i); + if (r != 0) + return r; - /* If we are not started from PID 1 we just inherit STDOUT from our parent process. */ - if (getppid() != 1) - return fileno; - - /* We need to open /dev/null here anew, to get the right access mode. */ - return open_null_as(O_WRONLY, fileno); + o = context->std_output; } switch (o) { From 7498c391eca1bb92db0feeeb92308cf02d575d4a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 21 Nov 2025 23:23:46 +0100 Subject: [PATCH 8/8] core/exec-invoke: remove redundant accmode validation on stderr Follow-up for 171ceb4a00294c700c0ba6906a3a3abad846699e In setup_output() we assume stdout has been set up properly before stderr, hence the stdout we're inheriting from must be writable (or more precisely, would have been adjusted to be). Hence no need to duplicate it. --- src/core/exec-invoke.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 7cc377a253..24e553e122 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -546,17 +546,8 @@ static int setup_output( return fileno; /* Duplicate from stdout if possible */ - if (can_inherit_stderr_from_stdout(context)) { - 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); - } + if (can_inherit_stderr_from_stdout(context)) return RET_NERRNO(dup2(STDOUT_FILENO, fileno)); - } o = context->std_error;