[Qemu-devel] [PATCH v2] console: fix cell overflow

Gerd Hoffmann posted 1 patch 4 years, 10 months ago
Test checkpatch passed
Test s390x failed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190701075301.14165-1-kraxel@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
ui/console.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] console: fix cell overflow
Posted by Gerd Hoffmann 4 years, 10 months ago
Linux terminal behavior (coming from vt100 I think) is somewhat strange
when it comes to line wraps:  When a character is printed to the last
char cell of a line the cursor does NOT jump to the next line but stays
where it is.  The line feed happens when the next character is printed.

So the valid range for the cursor position is not 0 .. width-1 but
0 .. width, where x == width represents the state where the line is
full but the cursor didn't jump to the next line yet.

The code for the 'clear from start of line' control sequence (ESC[1K)
fails to handle this corner case correctly and may call
console_clear_xy() with x == width.  That will incorrectly clear the
first char cell of the next line, or in case the cursor happens to be on
the last line overflow the cell buffer by one character (three bytes).

Add a check to the loop to fix that.

Didn't spot any other places with the same problem.  But it's easy to
miss that corner case, so also allocate one extra cell as precaution, so
in case we have simliar issues lurking elsewhere it at least wouldn't be
a buffer overflow.

v2: squashed in additional checks suggested by Christophe de Dinechin.

Reported-by: Alexander Oleinik <alxndr@bu.edu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
---
 ui/console.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index eb7e7e0c517a..82d1ddac9cfd 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -484,7 +484,7 @@ static void text_console_resize(QemuConsole *s)
     if (s->width < w1)
         w1 = s->width;
 
-    cells = g_new(TextCell, s->width * s->total_height);
+    cells = g_new(TextCell, s->width * s->total_height + 1);
     for(y = 0; y < s->total_height; y++) {
         c = &cells[y * s->width];
         if (w1 > 0) {
@@ -541,6 +541,9 @@ static void update_xy(QemuConsole *s, int x, int y)
         y2 += s->total_height;
     }
     if (y2 < s->height) {
+        if (x >= s->width) {
+            x = s->width - 1;
+        }
         c = &s->cells[y1 * s->width + x];
         vga_putcharxy(s, x, y2, c->ch,
                       &(c->t_attrib));
@@ -787,6 +790,9 @@ static void console_handle_escape(QemuConsole *s)
 static void console_clear_xy(QemuConsole *s, int x, int y)
 {
     int y1 = (s->y_base + y) % s->total_height;
+    if (x >= s->width) {
+        x = s->width - 1;
+    }
     TextCell *c = &s->cells[y1 * s->width + x];
     c->ch = ' ';
     c->t_attrib = s->t_attrib_default;
@@ -992,7 +998,7 @@ static void console_putchar(QemuConsole *s, int ch)
                     break;
                 case 1:
                     /* clear from beginning of line */
-                    for (x = 0; x <= s->x; x++) {
+                    for (x = 0; x <= s->x && x < s->width; x++) {
                         console_clear_xy(s, x, s->y);
                     }
                     break;
-- 
2.18.1