> On 27 Jun 2019, at 07:35, Gerd Hoffmann <kra...@redhat.com> wrote:
>
> 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.
>
> Reported-by: Alexander Oleinik <alx...@bu.edu>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> ui/console.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/console.c b/ui/console.c
> index eb7e7e0c517a..13d933510cdb 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);
I don’t like allocating just in case. At least put a comment explaining why ;-)
This extra byte only catches a single case (arguably an existing one).
What about adding a couple of extra tests where cell[… + x] is used?
(there is a third location I did not protect, because it already had
exactly that test)
diff --git a/ui/console.c b/ui/console.c
index eb7e7e0c51..00d27f6384 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -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;
Reviewed-by: Christophe de Dinechin <dinec...@redhat.com>
> for(y = 0; y < s->total_height; y++) {
> c = &cells[y * s->width];
> if (w1 > 0) {
> @@ -992,7 +992,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
>
>
