From d2b9e755242a98f14d89537380bef4e8e5ab9791 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Nov 2023 13:39:49 +0100 Subject: [PATCH 1/5] execute: add new helper exec_context_apply_tty_size() This combines exec_context_determine_tty_size() and terminal_set_size_fd() since we always use one after the other. Also make exec_context_determine_tty_size() return void, since it cannot fail. --- src/core/exec-invoke.c | 39 +++++++++++++++++---------------------- src/core/execute.c | 41 ++++++++++++++++++++++++++++------------- src/core/execute.h | 2 +- 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 38af3b488d..627b96d435 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -392,12 +392,9 @@ static int setup_input( /* Try to make this the controlling tty, if it is a tty, and reset it */ if (isatty(STDIN_FILENO)) { - unsigned rows = context->tty_rows, cols = context->tty_cols; - - (void) exec_context_tty_size(context, &rows, &cols); (void) ioctl(STDIN_FILENO, TIOCSCTTY, context->std_input == EXEC_INPUT_TTY_FORCE); (void) reset_terminal_fd(STDIN_FILENO, true); - (void) terminal_set_size_fd(STDIN_FILENO, NULL, rows, cols); + (void) exec_context_apply_tty_size(context, STDIN_FILENO, /* tty_path= */ NULL); } return STDIN_FILENO; @@ -413,26 +410,29 @@ static int setup_input( case EXEC_INPUT_TTY: case EXEC_INPUT_TTY_FORCE: case EXEC_INPUT_TTY_FAIL: { - unsigned rows, cols; - int fd; + _cleanup_close_ int tty_fd = -EBADF; + const char *tty_path; - fd = acquire_terminal(exec_context_tty_path(context), - i == EXEC_INPUT_TTY_FAIL ? ACQUIRE_TERMINAL_TRY : - i == EXEC_INPUT_TTY_FORCE ? ACQUIRE_TERMINAL_FORCE : - ACQUIRE_TERMINAL_WAIT, - USEC_INFINITY); - if (fd < 0) - return fd; + tty_path = ASSERT_PTR(exec_context_tty_path(context)); - r = exec_context_tty_size(context, &rows, &cols); + tty_fd = acquire_terminal(tty_path, + i == EXEC_INPUT_TTY_FAIL ? ACQUIRE_TERMINAL_TRY : + i == EXEC_INPUT_TTY_FORCE ? ACQUIRE_TERMINAL_FORCE : + ACQUIRE_TERMINAL_WAIT, + USEC_INFINITY); + if (tty_fd < 0) + return tty_fd; + + r = exec_context_apply_tty_size(context, tty_fd, tty_path); if (r < 0) return r; - r = terminal_set_size_fd(fd, exec_context_tty_path(context), rows, cols); + r = move_fd(tty_fd, STDIN_FILENO, /* cloexec= */ false); if (r < 0) return r; - return move_fd(fd, STDIN_FILENO, false); + TAKE_FD(tty_fd); + return r; } case EXEC_INPUT_SOCKET: @@ -692,7 +692,6 @@ static int setup_confirm_stdio( int *ret_saved_stdout) { _cleanup_close_ int fd = -EBADF, saved_stdin = -EBADF, saved_stdout = -EBADF; - unsigned rows, cols; int r; assert(ret_saved_stdin); @@ -718,11 +717,7 @@ static int setup_confirm_stdio( if (r < 0) return r; - r = exec_context_tty_size(context, &rows, &cols); - if (r < 0) - return r; - - r = terminal_set_size_fd(fd, vc, rows, cols); + r = exec_context_apply_tty_size(context, fd, vc); if (r < 0) return r; diff --git a/src/core/execute.c b/src/core/execute.c index e83a2ab239..61d0d721d7 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -99,27 +99,46 @@ const char *exec_context_tty_path(const ExecContext *context) { return "/dev/console"; } -int exec_context_tty_size(const ExecContext *context, unsigned *ret_rows, unsigned *ret_cols) { +static void exec_context_determine_tty_size( + const ExecContext *context, + const char *tty_path, + unsigned *ret_rows, + unsigned *ret_cols) { + unsigned rows, cols; - const char *tty; assert(context); assert(ret_rows); assert(ret_cols); + if (!tty_path) + tty_path = exec_context_tty_path(context); + rows = context->tty_rows; cols = context->tty_cols; - tty = exec_context_tty_path(context); - if (tty) - (void) proc_cmdline_tty_size(tty, rows == UINT_MAX ? &rows : NULL, cols == UINT_MAX ? &cols : NULL); + if (tty_path && (rows == UINT_MAX || cols == UINT_MAX)) + (void) proc_cmdline_tty_size( + tty_path, + rows == UINT_MAX ? &rows : NULL, + cols == UINT_MAX ? &cols : NULL); *ret_rows = rows; *ret_cols = cols; - - return 0; } +int exec_context_apply_tty_size( + const ExecContext *context, + int tty_fd, + const char *tty_path) { + + unsigned rows, cols; + + exec_context_determine_tty_size(context, tty_path, &rows, &cols); + + return terminal_set_size_fd(tty_fd, tty_path, rows, cols); + } + void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) { _cleanup_close_ int _fd = -EBADF, lock_fd = -EBADF; int fd; @@ -152,12 +171,8 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) if (context->tty_reset) (void) reset_terminal_fd(fd, true); - if (p && p->stdin_fd >= 0) { - unsigned rows = context->tty_rows, cols = context->tty_cols; - - (void) exec_context_tty_size(context, &rows, &cols); - (void) terminal_set_size_fd(p->stdin_fd, path, rows, cols); - } + if (p && p->stdin_fd >= 0) + (void) exec_context_apply_tty_size(context, p->stdin_fd, path); if (context->tty_vt_disallocate && path) (void) vt_disallocate(path); diff --git a/src/core/execute.h b/src/core/execute.h index 4896a42492..e15e19aae6 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -513,7 +513,7 @@ int exec_context_get_clean_directories(ExecContext *c, char **prefix, ExecCleanM int exec_context_get_clean_mask(ExecContext *c, ExecCleanMask *ret); const char *exec_context_tty_path(const ExecContext *context); -int exec_context_tty_size(const ExecContext *context, unsigned *ret_rows, unsigned *ret_cols); +int exec_context_apply_tty_size(const ExecContext *context, int tty_fd, const char *tty_path); void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p); uint64_t exec_context_get_rlimit(const ExecContext *c, const char *name); From bb2dbe7c6a951baaab5ea8c426d062c7f9823018 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Nov 2023 13:55:58 +0100 Subject: [PATCH 2/5] exec-invoke: only reset tty if we are told to We have a setting for this, honour it also if we are invoked with stdin passed in as fd. Fixes: #29714 --- src/core/exec-invoke.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 627b96d435..5ba0a47148 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -393,7 +393,10 @@ static int setup_input( /* Try to make this the controlling tty, if it is a tty, and reset it */ if (isatty(STDIN_FILENO)) { (void) ioctl(STDIN_FILENO, TIOCSCTTY, context->std_input == EXEC_INPUT_TTY_FORCE); - (void) reset_terminal_fd(STDIN_FILENO, true); + + if (context->tty_reset) + (void) reset_terminal_fd(STDIN_FILENO, /* switch_to_text= */ true); + (void) exec_context_apply_tty_size(context, STDIN_FILENO, /* tty_path= */ NULL); } From 29ed1f02034738d07d07d088fe1a9d48634cc4fb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Nov 2023 13:56:51 +0100 Subject: [PATCH 3/5] execute: make clear what reset_terminal_fd()'s 2nd param is about --- src/core/exec-invoke.c | 2 +- src/core/execute.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 5ba0a47148..1e08296b46 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -716,7 +716,7 @@ static int setup_confirm_stdio( if (r < 0) return r; - r = reset_terminal_fd(fd, true); + r = reset_terminal_fd(fd, /* switch_to_text= */ true); if (r < 0) return r; diff --git a/src/core/execute.c b/src/core/execute.c index 61d0d721d7..f3e895ea7f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -169,7 +169,7 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) (void) terminal_vhangup_fd(fd); if (context->tty_reset) - (void) reset_terminal_fd(fd, true); + (void) reset_terminal_fd(fd, /* switch_to_text= */ true); if (p && p->stdin_fd >= 0) (void) exec_context_apply_tty_size(context, p->stdin_fd, path); From c34eaeb5ebffff61157022fb618fd69c11a67aa6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Nov 2023 13:57:09 +0100 Subject: [PATCH 4/5] execute: use right fd to fix tty size --- src/core/execute.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index f3e895ea7f..e63137440a 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -171,8 +171,7 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *p) if (context->tty_reset) (void) reset_terminal_fd(fd, /* switch_to_text= */ true); - if (p && p->stdin_fd >= 0) - (void) exec_context_apply_tty_size(context, p->stdin_fd, path); + (void) exec_context_apply_tty_size(context, fd, path); if (context->tty_vt_disallocate && path) (void) vt_disallocate(path); From 561c1529be6220633c0948afc5009caf054526c6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Nov 2023 14:11:33 +0100 Subject: [PATCH 5/5] execute: add comment explaining what stdio_as_fds does --- src/core/execute.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/execute.h b/src/core/execute.h index e15e19aae6..5a6927aa02 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -224,7 +224,11 @@ struct ExecContext { ExecInput std_input; ExecOutput std_output; ExecOutput std_error; + + /* At least one of stdin/stdout/stderr was initialized from an fd passed in. This boolean survives + * the fds being closed. This only makes sense for transient units. */ bool stdio_as_fds; + char *stdio_fdname[3]; char *stdio_file[3];