From 1925593a374b3e3b4cead86abc4ae352260a372e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 9 Sep 2024 06:51:10 +0200 Subject: [PATCH] render: resize(): don't overflow the number of scrollback lines The config system allows setting the scrollback lines to 2**32-1. However, the total number of grid lines is the scrollback lines plus the window size, and then rounded *up* to the nearest power of two. Furthermore, the number of rows is represented with a plain 'int' throughout the code base. The largest positive integer that fits in an int is 2**31-1. That however, is not a power of two. The largest positive integer, that also is a power of two, that fits in an int is 2**30, or 1073741824. Ideally, we'd just cast the line count to a 64-bit integer, and call __builtin_clzl{,l}() on it, and then take the smallest value of that, or 2**30. But, for some reason, __builtin_clzl(), and __builtin_clzll() appears to ignore bits above 32, despite they being typed to long and long long. Bug? Instead, ensure we never call __builtin_clz() on anything larger than 2**30. Closes #1828 --- CHANGELOG.md | 5 +++++ render.c | 33 +++++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c155f362..d8cadf79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,11 @@ * Some invalid UTF-8 strings passing the validity check when setting the window title, triggering a Wayland protocol error which then caused foot to shutdown. +* "Too large" values for `scrollback.lines` causing an integer + overflow, resulting in either visual glitches, crashes, or both + ([#1828][1828]). + +[1828]: https://codeberg.org/dnkl/foot/issues/1828 ### Security diff --git a/render.c b/render.c index 98bc0b69..00ea3445 100644 --- a/render.c +++ b/render.c @@ -4369,8 +4369,6 @@ render_resize(struct terminal *term, int width, int height, uint8_t opts) term->width = width; term->height = height; - const uint32_t scrollback_lines = term->render.scrollback_lines; - /* Screen rows/cols before resize */ int old_cols = term->cols; int old_rows = term->rows; @@ -4379,9 +4377,36 @@ render_resize(struct terminal *term, int width, int height, uint8_t opts) const int new_cols = (term->width - 2 * pad_x) / term->cell_width; const int new_rows = (term->height - 2 * pad_y) / term->cell_height; + /* + * Requirements for scrollback: + * + * a) total number of rows (visible + scrollback history) must be + * a power of two + * b) must be representable in a plain int (signed) + * + * This means that on a "normal" system, where ints are 32-bit, + * the largest possible scrollback size is 1073741824 (0x40000000, + * 1 << 30). + * + * The largest *signed* int is 2147483647 (0x7fffffff), which is + * *not* a power of two. + * + * Note that these are theoretical limits. Most of the time, + * you'll get a memory allocation failure when trying to allocate + * the grid array. + */ + const unsigned max_scrollback = (INT_MAX >> 1) + 1; + const unsigned scrollback_lines_not_yet_power_of_two = + min((uint64_t)term->render.scrollback_lines + new_rows - 1, max_scrollback); + /* Grid rows/cols after resize */ - const int new_normal_grid_rows = 1 << (32 - __builtin_clz(new_rows + scrollback_lines - 1)); - const int new_alt_grid_rows = 1 << (32 - __builtin_clz(new_rows)); + const int new_normal_grid_rows = + min(1u << (32 - __builtin_clz(scrollback_lines_not_yet_power_of_two)), + max_scrollback); + const int new_alt_grid_rows = + min(1u << (32 - __builtin_clz(new_rows)), max_scrollback); + + LOG_DBG("grid rows: %d", new_normal_grid_rows); xassert(new_cols >= 1); xassert(new_rows >= 1);