[PATCH] vt: Handle recursion in vc_do_resize().
STALE2253d
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: 2020-07-29 05:30:53
Also in:
dri-devel, lkml
Subsystem:
the rest, tty layer and serial drivers · Maintainers:
Linus Torvalds, Greg Kroah-Hartman, Jiri Slaby
syzbot is reporting OOB read bug in vc_do_resize() [1] caused by memcpy()
based on outdated old_{rows,row_size} values, for resize_screen() can
recurse into vc_do_resize() which changes vc->vc_{cols,rows} that outdates
old_{rows,row_size} values which were read before calling resize_screen().
Minimal fix might be to read vc->vc_{rows,size_row} after resize_screen().
A different fix might be to forbid recursive vc_do_resize() request.
I can't tell which fix is the better.
But since I guess that new_cols = vc->vc_cols && new_rows = vc->vc_rows
check could become true after returning from resize_screen(), and I assume
that not calling clear_selection() when resize_screen() will return error
is harmless, let's redo the check by moving resize_screen() earlier.
[1] https://syzkaller.appspot.com/bug?idÇ0c88cfd16dcf6e1d3c7f0ab8648b3144b5b25e
Reported-by: syzbot <redacted>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/tty/vt/vt.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 42d8c67..952a067 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c@@ -1217,7 +1217,24 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc, if (new_cols = vc->vc_cols && new_rows = vc->vc_rows) return 0; + if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size) + return -EINVAL; + /* + * Since fbcon_resize() from resize_screen() can recurse into + * this function via fb_set_var(), handle recursion now. + */ + err = resize_screen(vc, new_cols, new_rows, user); + if (err) + return err; + /* Reload values in case recursion changed vc->vc_{cols,rows}. */ + new_cols = (cols ? cols : vc->vc_cols); + new_rows = (lines ? lines : vc->vc_rows); + new_row_size = new_cols << 1; + new_screen_size = new_row_size * new_rows; + + if (new_cols = vc->vc_cols && new_rows = vc->vc_rows) + return 0; if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size) return -EINVAL; newscreen = kzalloc(new_screen_size, GFP_USER);
@@ -1238,13 +1255,6 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc, old_rows = vc->vc_rows; old_row_size = vc->vc_size_row; - err = resize_screen(vc, new_cols, new_rows, user); - if (err) { - kfree(newscreen); - vc_uniscr_free(new_uniscr); - return err; - } - vc->vc_rows = new_rows; vc->vc_cols = new_cols; vc->vc_size_row = new_row_size;
--
1.8.3.1