From 933c6900308fe321b8f7aa765df293969d81e399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Nov 2025 14:27:57 +0100 Subject: [PATCH 1/3] basic/terminal-util: operate on one fd in terminal_get_size_by_dsr() This moves the open call earlier, so that we do any state-changing operations if we actually managed to open the nonblocking fd. This makes the code more robust because if the fdreopen call fails, we won't make modifications to the state of the terminal. --- src/basic/terminal-util.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index b7de72daed..f17352b41b 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -2369,7 +2369,6 @@ int terminal_get_size_by_dsr( unsigned *ret_rows, unsigned *ret_columns) { - _cleanup_close_ int nonblock_input_fd = -EBADF; int r; assert(input_fd >= 0); @@ -2397,14 +2396,20 @@ int terminal_get_size_by_dsr( if (r < 0) return log_debug_errno(r, "Called with distinct input/output fds: %m"); + /* Open a 2nd input fd, in non-blocking mode, so that we won't ever hang in read() + * should someone else process the POLLIN. Do all subsequent operations on the new fd. */ + _cleanup_close_ int nonblock_input_fd = r = fd_reopen(input_fd, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); + if (r < 0) + return r; + struct termios old_termios; - if (tcgetattr(input_fd, &old_termios) < 0) + if (tcgetattr(nonblock_input_fd, &old_termios) < 0) return log_debug_errno(errno, "Failed to get terminal settings: %m"); struct termios new_termios = old_termios; termios_disable_echo(&new_termios); - if (tcsetattr(input_fd, TCSANOW, &new_termios) < 0) + if (tcsetattr(nonblock_input_fd, TCSANOW, &new_termios) < 0) return log_debug_errno(errno, "Failed to set new terminal settings: %m"); unsigned saved_row = 0, saved_column = 0; @@ -2417,13 +2422,6 @@ int terminal_get_size_by_dsr( if (r < 0) goto finish; - /* Open a 2nd input fd, in non-blocking mode, so that we won't ever hang in read() should someone - * else process the POLLIN. */ - - nonblock_input_fd = r = fd_reopen(input_fd, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); - if (r < 0) - goto finish; - usec_t end = usec_add(now(CLOCK_MONOTONIC), CONSOLE_REPLY_WAIT_USEC); char buf[STRLEN("\x1B[1;1R")]; /* The shortest valid reply possible */ size_t buf_full = 0; @@ -2516,7 +2514,7 @@ finish: if (saved_row > 0 && saved_column > 0) RET_GATHER(r, terminal_set_cursor_position(output_fd, saved_row, saved_column)); - RET_GATHER(r, RET_NERRNO(tcsetattr(input_fd, TCSANOW, &old_termios))); + RET_GATHER(r, RET_NERRNO(tcsetattr(nonblock_input_fd, TCSANOW, &old_termios))); return r; } From 46f6742911ba70ad19cc94da241dc4ff7b1c7489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Nov 2025 14:33:26 +0100 Subject: [PATCH 2/3] basic/terminal-util: operate on one fd in get_default_background_color() This moves the open call earlier, so that we do any state-changing operations if we actually managed to open the nonblocking fd. The code is easier to follow this way and might be more robust. Suprisingly, this fixes https://github.com/systemd/systemd/issues/39055: it seems that run0 chowns /dev/stdin (in my case /dev/pts/0) to root:root, and the second run0 can read and write stdin/stdout throught the already-open fds, but fd_reopen fails. Fixes https://github.com/systemd/systemd/issues/39055. --- src/basic/terminal-util.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index f17352b41b..337c35a193 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -2260,7 +2260,6 @@ static int scan_background_color_response( } int get_default_background_color(double *ret_red, double *ret_green, double *ret_blue) { - _cleanup_close_ int nonblock_input_fd = -EBADF; int r; assert(ret_red); @@ -2280,27 +2279,26 @@ int get_default_background_color(double *ret_red, double *ret_green, double *ret return 0; } + /* Open a 2nd input fd, in non-blocking mode, so that we won't ever hang in read() + * should someone else process the POLLIN. Do all subsequent operations on the new fd. */ + _cleanup_close_ int nonblock_input_fd = r = fd_reopen(STDIN_FILENO, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); + if (r < 0) + return r; + struct termios old_termios; - if (tcgetattr(STDIN_FILENO, &old_termios) < 0) + if (tcgetattr(nonblock_input_fd, &old_termios) < 0) return -errno; struct termios new_termios = old_termios; termios_disable_echo(&new_termios); - if (tcsetattr(STDIN_FILENO, TCSANOW, &new_termios) < 0) + if (tcsetattr(nonblock_input_fd, TCSANOW, &new_termios) < 0) return -errno; r = loop_write(STDOUT_FILENO, ANSI_OSC "11;?" ANSI_ST, SIZE_MAX); if (r < 0) goto finish; - /* Open a 2nd input fd, in non-blocking mode, so that we won't ever hang in read() should someone - * else process the POLLIN. */ - - nonblock_input_fd = r = fd_reopen(STDIN_FILENO, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); - if (r < 0) - goto finish; - usec_t end = usec_add(now(CLOCK_MONOTONIC), CONSOLE_REPLY_WAIT_USEC); char buf[STRLEN(ANSI_OSC "11;rgb:0/0/0" ANSI_ST)]; /* shortest possible reply */ size_t buf_full = 0; @@ -2359,7 +2357,7 @@ int get_default_background_color(double *ret_red, double *ret_green, double *ret } finish: - RET_GATHER(r, RET_NERRNO(tcsetattr(STDIN_FILENO, TCSANOW, &old_termios))); + RET_GATHER(r, RET_NERRNO(tcsetattr(nonblock_input_fd, TCSANOW, &old_termios))); return r; } From e698ee5705758cb0fd8ad0186db47541c6c4ea5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Nov 2025 16:37:30 +0100 Subject: [PATCH 3/3] basic/terminal-util: ignore failures in cleanup Some of the functions were ignoring failure in cleanup, others weren't. If we got a reply, it's better to use it, so ignore failures in cleanup everywhere. --- src/basic/terminal-util.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 337c35a193..5609715b8a 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -2071,7 +2071,9 @@ int terminal_get_cursor_position( } finish: - RET_GATHER(r, RET_NERRNO(tcsetattr(input_fd, TCSANOW, &old_termios))); + /* We ignore failure here and in similar cases below. We already got a reply and if cleanup fails, + * this doesn't change the validity of the result. */ + (void) tcsetattr(input_fd, TCSANOW, &old_termios); return r; } @@ -2357,7 +2359,7 @@ int get_default_background_color(double *ret_red, double *ret_green, double *ret } finish: - RET_GATHER(r, RET_NERRNO(tcsetattr(nonblock_input_fd, TCSANOW, &old_termios))); + (void) tcsetattr(nonblock_input_fd, TCSANOW, &old_termios); return r; } @@ -2510,9 +2512,9 @@ int terminal_get_size_by_dsr( finish: /* Restore cursor position */ if (saved_row > 0 && saved_column > 0) - RET_GATHER(r, terminal_set_cursor_position(output_fd, saved_row, saved_column)); + (void) terminal_set_cursor_position(output_fd, saved_row, saved_column); + (void) tcsetattr(nonblock_input_fd, TCSANOW, &old_termios); - RET_GATHER(r, RET_NERRNO(tcsetattr(nonblock_input_fd, TCSANOW, &old_termios))); return r; } @@ -2665,7 +2667,6 @@ int terminal_get_terminfo_by_dcs(int fd, char **ret_name) { } finish: - /* We ignore failure here. We already got a reply and if cleanup fails, we can't help that. */ (void) tcsetattr(fd, TCSANOW, &old_termios); return r; }