[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200710055329.3759-1-penguin-kernel@I-love.SAKURA.ne.jp>
Date: Fri, 10 Jul 2020 14:53:29 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>
Cc: Dmitry Vyukov <dvyukov@...gle.com>, linux-kernel@...r.kernel.org,
Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
syzbot <syzbot+017265e8553724e514e8@...kaller.appspotmail.com>
Subject: [PATCH] vt: Reject zero-sized screen buffer size.
syzbot is reporting general protection fault in do_con_write() [1] caused
by vc->vc_screenbuf == ZERO_SIZE_PTR caused by vc->vc_screenbuf_size == 0
caused by vc->vc_cols == vc->vc_rows == vc->vc_size_row == 0 being passed
to ioctl(FBIOPUT_VSCREENINFO) request on /dev/fb0 , for gotoxy(vc, 0, 0)
from reset_terminal() from vc_init() from vc_allocate() on such console
causes vc->vc_pos == 0x10000000e due to
((unsigned long) ZERO_SIZE_PTR) + -1U * 0 + (-1U << 1).
I don't think that a console with 0 column and/or 0 row makes sense, and
I think that we can reject such bogus arguments in fb_set_var() from
ioctl(FBIOPUT_VSCREENINFO). Regardless, I think that it is safer to also
check ZERO_SIZE_PTR when allocating vc->vc_screenbuf from vc_allocate()
from con_install() from tty_init_dev() from tty_open().
[1] https://syzkaller.appspot.com/bug?extid=017265e8553724e514e8
Reported-by: syzbot <syzbot+017265e8553724e514e8@...kaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
---
drivers/tty/vt/vt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 48a8199f7845..8497e9206607 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1126,7 +1126,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
con_set_default_unimap(vc);
vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL);
- if (!vc->vc_screenbuf)
+ if (ZERO_OR_NULL_PTR(vc->vc_screenbuf))
goto err_free;
/* If no drivers have overridden us and the user didn't pass a
@@ -1212,7 +1212,7 @@ 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)
+ if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size)
return -EINVAL;
newscreen = kzalloc(new_screen_size, GFP_USER);
if (!newscreen)
@@ -3393,6 +3393,7 @@ static int __init con_init(void)
INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
tty_port_init(&vc->port);
visual_init(vc, currcons, 1);
+ /* Assuming vc->vc_screenbuf_size is sane here, for this is __init code. */
vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
vc_init(vc, vc->vc_rows, vc->vc_cols,
currcons || !vc->vc_sw->con_save_screen);
--
2.18.4
Powered by blists - more mailing lists