From abca10b766a45c27426894e960fd313cad862a82 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 28 Mar 2026 14:10:54 +0000 Subject: [PATCH] terminal-util: fix boot hang from ANSI terminal size queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since v257, terminal_fix_size() is called during early boot via console_setup() → reset_dev_console_fd() to query terminal dimensions via ANSI escape sequences. This has caused intermittent boot hangs where the system gets stuck with a blinking cursor and requires a keypress to continue (see systemd/systemd#35499). The function tries CSI 18 first, then falls back to DSR if that fails. Previously, each method independently opened a non-blocking fd, disabled echo/icanon, ran its query, restored termios, and closed its fd. This created two problems: 1. Echo window between CSI 18 and DSR fallback: After CSI 18 times out and restores termios (re-enabling ECHO and ICANON), there is a brief window before DSR disables them again. If the terminal's CSI 18 response arrives during this window, it is echoed back to the terminal — where the terminal interprets \e[8;rows;cols t as a "resize text area" command — and the response bytes land in the canonical line buffer as stale input that can confuse the DSR response parser. 2. Cursor left at bottom-right on DSR timeout: The DSR method worked by sending two DSR queries — one to save the cursor position, then moving the cursor to (32766,32766) and sending another to read the clamped position. If neither response was received (timeout), the cursor restore was skipped (conditional on saved_row > 0), leaving the cursor at the bottom-right corner of the terminal. The subsequent terminal_reset_ansi_seq() then moved it to the beginning of the last line via \e[1G, making boot output appear at the bottom of the screen — giving the appearance of a hang even when the system was still booting. This commit fixes both issues: - terminal_fix_size() now opens the non-blocking fd and configures termios once for both query methods, so echo stays disabled for the entire CSI 18 → DSR fallback sequence with no gap. tcflush(TCIFLUSH) is called before each query to drain any stale input from the tty input queue. - The DSR method now uses DECSC (\e7) / DECRC (\e8) to save and restore the cursor position via hardware, instead of querying it with a separate DSR round-trip. All four sequences (DECSC, CUP to bottom-right, DSR query, DECRC) are sent in a single write, so the terminal processes DECRC and restores the cursor regardless of whether userspace ever reads the DSR response. This eliminates the cursor-at-bottom-right artifact on timeout and simplifies the read loop to only need a single DSR response instead of two. - The repeated setup boilerplate (dumb check, verify_same, fd_reopen, termios save/disable) is extracted into terminal_prepare_query(), shared by terminal_get_size_by_csi18(), terminal_get_size_by_dsr(), and terminal_fix_size(). Fixes: systemd/systemd#35499 Co-developed-by: Claude Opus 4.6 (cherry picked from commit da69848791d2b32dfb90946264fd632ac1d5c7de) --- src/basic/terminal-util.c | 300 ++++++++++++++++------------------ src/basic/terminal-util.h | 5 +- src/core/execute.c | 2 +- src/test/test-terminal-util.c | 8 +- 4 files changed, 148 insertions(+), 167 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 0ddaa111913..f90da747848 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -2367,26 +2367,105 @@ int get_default_background_color(double *ret_red, double *ret_green, double *ret } } -int terminal_get_size_by_dsr( - int input_fd, +/* Determine terminal dimensions by means of ANSI sequences: save the cursor via DECSC, position it far + * to the bottom right (clamped to actual terminal dimensions), read back via DSR where we ended up, and + * restore cursor via DECRC. Only needs a single DSR round-trip, and always restores the cursor regardless + * of whether the response is received. + * + * Caller must have already opened a non-blocking input fd and configured termios (echo/icanon off). */ +static int terminal_query_size_by_dsr( + int nonblock_input_fd, int output_fd, unsigned *ret_rows, unsigned *ret_columns) { int r; - assert(input_fd >= 0); + assert(nonblock_input_fd >= 0); assert(output_fd >= 0); - /* Tries to determine the terminal dimension by means of ANSI sequences. - * - * We position the cursor briefly at an absolute location very far down and very far to the right, - * and then read back where we actually ended up. Because cursor locations are capped at the terminal - * width/height we should then see the right values. In order to not risk integer overflows in - * terminal applications we'll use INT16_MAX-1 as location to jump to — hopefully a value that is - * large enough for any real-life terminals, but small enough to not overflow anything or be - * recognized as a "niche" value. (Note that the dimension fields in "struct winsize" are 16bit only, - * too). */ + /* Use DECSC/DECRC to save/restore cursor instead of querying position via DSR. This way the cursor + * is always restored — even on timeout — and we only need one DSR response instead of two. */ + r = loop_write(output_fd, + "\x1B" "7" /* DECSC: save cursor position */ + "\x1B[32766;32766H" /* CUP: position cursor far to the right and to the bottom, staying within 16bit signed range */ + "\x1B[6n" /* DSR: request cursor position (CPR) */ + "\x1B" "8", /* DECRC: restore cursor position */ + SIZE_MAX); + if (r < 0) + return r; + + 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; + CursorPositionContext context = {}; + + for (bool first = true;; first = false) { + if (buf_full == 0) { + usec_t n = now(CLOCK_MONOTONIC); + if (n >= end) + return -EOPNOTSUPP; + + r = fd_wait_for_event(nonblock_input_fd, POLLIN, usec_sub_unsigned(end, n)); + if (r < 0) + return r; + if (r == 0) + return -EOPNOTSUPP; + + /* On the first try, read multiple characters, i.e. the shortest valid + * reply. Afterwards read byte-wise, since we don't want to read too much, and + * unnecessarily drop too many characters from the input queue. */ + ssize_t l = read(nonblock_input_fd, buf, first ? sizeof(buf) : 1); + if (l < 0) { + if (errno == EAGAIN) + continue; + + return -errno; + } + + assert((size_t) l <= sizeof(buf)); + buf_full = l; + } + + size_t processed; + r = scan_cursor_position_response(&context, buf, buf_full, &processed); + if (r < 0) + return r; + + assert(processed <= buf_full); + buf_full -= processed; + memmove(buf, buf + processed, buf_full); + + if (r > 0) { + /* Superficial validity checks (no particular reason to check for < 4, it's + * just a way to look for unreasonably small values) */ + if (context.row < 4 || context.column < 4 || context.row >= 32766 || context.column >= 32766) + return -ENODATA; + + if (ret_rows) + *ret_rows = context.row; + if (ret_columns) + *ret_columns = context.column; + + return 0; + } + } +} + +/* Common setup for ANSI terminal queries: validate the fds, open a non-blocking input fd, and configure + * termios with echo and canonical mode disabled. Caller must restore termios and close the fd when done. */ +static int terminal_prepare_query( + int input_fd, + int output_fd, + int *ret_nonblock_fd, + struct termios *ret_saved_termios) { + + int r; + + assert(input_fd >= 0); + assert(output_fd >= 0); + assert(ret_nonblock_fd); + assert(ret_saved_termios); if (terminal_is_dumb()) return -EOPNOTSUPP; @@ -2401,121 +2480,17 @@ int terminal_get_size_by_dsr( if (r < 0) return r; - struct termios old_termios = TERMIOS_NULL; - CLEANUP_TERMIOS_RESET(nonblock_input_fd, old_termios); - - if (tcgetattr(nonblock_input_fd, &old_termios) < 0) + if (tcgetattr(nonblock_input_fd, ret_saved_termios) < 0) return log_debug_errno(errno, "Failed to get terminal settings: %m"); - struct termios new_termios = old_termios; + struct termios new_termios = *ret_saved_termios; termios_disable_echo(&new_termios); 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; - - r = loop_write(output_fd, - "\x1B[6n" /* Request cursor position (DSR/CPR) */ - "\x1B[32766;32766H" /* Position cursor really far to the right and to the bottom, but let's stay within the 16bit signed range */ - "\x1B[6n", /* Request cursor position again */ - SIZE_MAX); - 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; - CursorPositionContext context = {}; - - for (bool first = true;; first = false) { - if (buf_full == 0) { - usec_t n = now(CLOCK_MONOTONIC); - if (n >= end) { - r = -EOPNOTSUPP; - goto finish; - } - - r = fd_wait_for_event(nonblock_input_fd, POLLIN, usec_sub_unsigned(end, n)); - if (r < 0) - goto finish; - if (r == 0) { - r = -EOPNOTSUPP; - goto finish; - } - - /* On the first try, read multiple characters, i.e. the shortest valid - * reply. Afterwards read byte-wise, since we don't want to read too much, and - * unnecessarily drop too many characters from the input queue. */ - ssize_t l = read(nonblock_input_fd, buf, first ? sizeof(buf) : 1); - if (l < 0) { - if (errno == EAGAIN) - continue; - - r = -errno; - goto finish; - } - - assert((size_t) l <= sizeof(buf)); - buf_full = l; - } - - size_t processed; - r = scan_cursor_position_response(&context, buf, buf_full, &processed); - if (r < 0) - goto finish; - - assert(processed <= buf_full); - buf_full -= processed; - memmove(buf, buf + processed, buf_full); - - if (r > 0) { - if (saved_row == 0) { - assert(saved_column == 0); - - /* First sequence, this is the cursor position before we set it somewhere - * into the void at the bottom right. Let's save where we are so that we can - * return later. */ - - /* Superficial validity checks */ - if (context.row <= 0 || context.column <= 0 || context.row >= 32766 || context.column >= 32766) { - r = -ENODATA; - goto finish; - } - - saved_row = context.row; - saved_column = context.column; - - /* Reset state */ - context = (CursorPositionContext) {}; - } else { - /* Second sequence, this is the cursor position after we set it somewhere - * into the void at the bottom right. */ - - /* Superficial validity checks (no particular reason to check for < 4, it's - * just a way to look for unreasonably small values) */ - if (context.row < 4 || context.column < 4 || context.row >= 32766 || context.column >= 32766) { - r = -ENODATA; - goto finish; - } - - if (ret_rows) - *ret_rows = context.row; - if (ret_columns) - *ret_columns = context.column; - - r = 0; - goto finish; - } - } - } - -finish: - /* Restore cursor position */ - if (saved_row > 0 && saved_column > 0) - (void) terminal_set_cursor_position(output_fd, saved_row, saved_column); - - return r; + *ret_nonblock_fd = TAKE_FD(nonblock_input_fd); + return 0; } /* @@ -2554,43 +2529,20 @@ static int scan_text_area_size_response( return 0; } -int terminal_get_size_by_csi18( - int input_fd, +/* Determine terminal dimensions by means of an ANSI CSI 18 sequence. + * + * Caller must have already opened a non-blocking input fd and configured termios (echo/icanon off). */ +static int terminal_query_size_by_csi18( + int nonblock_input_fd, int output_fd, unsigned *ret_rows, unsigned *ret_columns) { + int r; - assert(input_fd >= 0); + assert(nonblock_input_fd >= 0); assert(output_fd >= 0); - /* Tries to determine the terminal dimension by means of an ANSI sequence CSI 18. */ - - if (terminal_is_dumb()) - return -EOPNOTSUPP; - - r = terminal_verify_same(input_fd, output_fd); - 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 = TERMIOS_NULL; - CLEANUP_TERMIOS_RESET(nonblock_input_fd, old_termios); - - 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(nonblock_input_fd, TCSANOW, &new_termios) < 0) - return log_debug_errno(errno, "Failed to set new terminal settings: %m"); - r = loop_write(output_fd, CSI18_Q, SIZE_MAX); if (r < 0) return r; @@ -2635,6 +2587,44 @@ int terminal_get_size_by_csi18( } } +int terminal_get_size( + int input_fd, + int output_fd, + unsigned *ret_rows, + unsigned *ret_columns, + bool try_dsr, + bool try_csi18) { + + _cleanup_close_ int nonblock_input_fd = -EBADF; + struct termios old_termios = TERMIOS_NULL; + CLEANUP_TERMIOS_RESET(nonblock_input_fd, old_termios); + int r; + + assert(try_dsr || try_csi18); + + r = terminal_prepare_query(input_fd, output_fd, &nonblock_input_fd, &old_termios); + if (r < 0) + return r; + + /* Flush any stale input that might confuse the response parsers. */ + (void) tcflush(nonblock_input_fd, TCIFLUSH); + + if (try_csi18) { + r = terminal_query_size_by_csi18(nonblock_input_fd, output_fd, ret_rows, ret_columns); + if (!IN_SET(r, -EOPNOTSUPP, -EINVAL) || !try_dsr) + return r; + + /* CSI 18 query failed. Flush input before trying the DSR fallback — a late CSI 18 response + * may have landed in the input queue and would confuse the DSR response parser. */ + (void) tcflush(nonblock_input_fd, TCIFLUSH); + } + + if (try_dsr) + r = terminal_query_size_by_dsr(nonblock_input_fd, output_fd, ret_rows, ret_columns); + + return r; +} + int terminal_fix_size(int input_fd, int output_fd) { unsigned rows, columns; int r; @@ -2646,20 +2636,12 @@ int terminal_fix_size(int input_fd, int output_fd) { * sequences are interpreted by the final terminal instead of an intermediary tty driver they should * be more accurate. */ - r = terminal_verify_same(input_fd, output_fd); - if (r < 0) - return r; struct winsize ws = {}; if (ioctl(output_fd, TIOCGWINSZ, &ws) < 0) return log_debug_errno(errno, "Failed to query terminal dimensions, ignoring: %m"); - r = terminal_get_size_by_csi18(input_fd, output_fd, &rows, &columns); - if (IN_SET(r, -EOPNOTSUPP, -EINVAL)) - /* We get -EOPNOTSUPP if the query fails and -EINVAL when the received answer is invalid. - * Try the fallback method. It is more involved and moves the cursor, but seems to have wider - * support. */ - r = terminal_get_size_by_dsr(input_fd, output_fd, &rows, &columns); + r = terminal_get_size(input_fd, output_fd, &rows, &columns, /* try_dsr= */ true, /* try_csi18= */ true); if (r < 0) return log_debug_errno(r, "Failed to acquire terminal dimensions via ANSI sequences, not adjusting terminal dimensions: %m"); diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index 6f29fa73a6f..dde1430243c 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -146,7 +146,7 @@ assert_cc((TTY_MODE & 0711) == 0600); void termios_disable_echo(struct termios *termios); /* A termios sentinel with all flag fields set to all-ones-bits. No real tcgetattr() result will ever - * match this because the multi-bit sub-fields (CSIZE, CBAUD, …) can't validly have every bit set. */ + * match this because no real terminal configuration uses all-ones in every flag field simultaneously. */ #define TERMIOS_NULL (struct termios) { \ .c_iflag = UINT_MAX, \ .c_oflag = UINT_MAX, \ @@ -172,8 +172,7 @@ void termios_reset(const TermiosResetContext *c); #define FALLBACK_TERM "vt220" int get_default_background_color(double *ret_red, double *ret_green, double *ret_blue); -int terminal_get_size_by_dsr(int input_fd, int output_fd, unsigned *ret_rows, unsigned *ret_columns); -int terminal_get_size_by_csi18(int input_fd, int output_fd, unsigned *ret_rows, unsigned *ret_columns); +int terminal_get_size(int input_fd, int output_fd, unsigned *ret_rows, unsigned *ret_columns, bool try_dsr, bool try_csi18); int terminal_fix_size(int input_fd, int output_fd); int terminal_get_terminfo_by_dcs(int fd, char **ret_name); diff --git a/src/core/execute.c b/src/core/execute.c index 5a47a8ccff4..54fcdac8d67 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -113,7 +113,7 @@ int exec_context_apply_tty_size( if (rows == UINT_MAX && cols == UINT_MAX && exec_context_shall_ansi_seq_reset(context) && isatty_safe(input_fd)) { - r = terminal_get_size_by_dsr(input_fd, output_fd, &rows, &cols); + r = terminal_get_size(input_fd, output_fd, &rows, &cols, /* try_dsr= */ true, /* try_csi18= */ false); if (r < 0) log_debug_errno(r, "Failed to get terminal size by DSR, ignoring: %m"); } diff --git a/src/test/test-terminal-util.c b/src/test/test-terminal-util.c index 18b2a63f7bb..909dda73a99 100644 --- a/src/test/test-terminal-util.c +++ b/src/test/test-terminal-util.c @@ -175,12 +175,12 @@ TEST(get_default_background_color) { log_notice("R=%g G=%g B=%g", red, green, blue); } -TEST(terminal_get_size_by_csi18) { +TEST(terminal_get_size_csi18) { unsigned rows, columns; int r; usec_t n = now(CLOCK_MONOTONIC); - r = terminal_get_size_by_csi18(STDIN_FILENO, STDOUT_FILENO, &rows, &columns); + r = terminal_get_size(STDIN_FILENO, STDOUT_FILENO, &rows, &columns, /* try_dsr= */ false, /* try_csi18= */ true); log_info("%s took %s", __func__+5, FORMAT_TIMESPAN(usec_sub_unsigned(now(CLOCK_MONOTONIC), n), USEC_PER_MSEC)); if (r < 0) @@ -196,12 +196,12 @@ TEST(terminal_get_size_by_csi18) { log_notice("terminal size via ioctl: rows=%u columns=%u", ws.ws_row, ws.ws_col); } -TEST(terminal_get_size_by_dsr) { +TEST(terminal_get_size_dsr) { unsigned rows, columns; int r; usec_t n = now(CLOCK_MONOTONIC); - r = terminal_get_size_by_dsr(STDIN_FILENO, STDOUT_FILENO, &rows, &columns); + r = terminal_get_size(STDIN_FILENO, STDOUT_FILENO, &rows, &columns, /* try_dsr= */ true, /* try_csi18= */ false); log_info("%s took %s", __func__+5, FORMAT_TIMESPAN(usec_sub_unsigned(now(CLOCK_MONOTONIC), n), USEC_PER_MSEC)); if (r < 0)