Skip to content

Commit aea2558

Browse files
Tetsuo Handakelmously
authored andcommitted
vt: Reject zero-sized screen buffer size.
BugLink: https://bugs.launchpad.net/bugs/1889669 commit ce68455 upstream. 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 caused by fb_set_var() from ioctl(FBIOPUT_VSCREENINFO) on /dev/fb0 , for gotoxy(vc, 0, 0) from reset_terminal() from vc_init() from vc_allocate() from con_install() from tty_init_dev() from tty_open() 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 or 0 row makes sense. And it seems that vc_do_resize() does not intend to allow resizing a console to 0 column or 0 row due to new_cols = (cols ? cols : vc->vc_cols); new_rows = (lines ? lines : vc->vc_rows); exception. Theoretically, cols and rows can be any range as long as 0 < cols * rows * 2 <= KMALLOC_MAX_SIZE is satisfied (e.g. cols == 1048576 && rows == 2 is possible) because of vc->vc_size_row = vc->vc_cols << 1; vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; in visual_init() and kzalloc(vc->vc_screenbuf_size) in vc_allocate(). Since we can detect cols == 0 or rows == 0 via screenbuf_size = 0 in visual_init(), we can reject kzalloc(0). Then, vc_allocate() will return an error, and con_write() will not be called on a console with 0 column or 0 row. We need to make sure that integer overflow in visual_init() won't happen. Since vc_do_resize() restricts cols <= 32767 and rows <= 32767, applying 1 <= cols <= 32767 and 1 <= rows <= 32767 restrictions to vc_allocate() will be practically fine. This patch does not touch con_init(), for returning -EINVAL there does not help when we are not returning -ENOMEM. [1] https://syzkaller.appspot.com/bug?extid=017265e8553724e514e8 Reported-and-tested-by: syzbot <[email protected]> Signed-off-by: Tetsuo Handa <[email protected]> Cc: stable <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Kamal Mostafa <[email protected]> Signed-off-by: Kelsey Skunberg <[email protected]>
1 parent 1853779 commit aea2558

File tree

1 file changed

+18
-11
lines changed

1 file changed

+18
-11
lines changed

drivers/tty/vt/vt.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,10 +1103,19 @@ static const struct tty_port_operations vc_port_ops = {
11031103
.destruct = vc_port_destruct,
11041104
};
11051105

1106+
/*
1107+
* Change # of rows and columns (0 means unchanged/the size of fg_console)
1108+
* [this is to be used together with some user program
1109+
* like resize that changes the hardware videomode]
1110+
*/
1111+
#define VC_MAXCOL (32767)
1112+
#define VC_MAXROW (32767)
1113+
11061114
int vc_allocate(unsigned int currcons) /* return 0 on success */
11071115
{
11081116
struct vt_notifier_param param;
11091117
struct vc_data *vc;
1118+
int err;
11101119

11111120
WARN_CONSOLE_UNLOCKED();
11121121

@@ -1136,6 +1145,11 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
11361145
if (!*vc->vc_uni_pagedir_loc)
11371146
con_set_default_unimap(vc);
11381147

1148+
err = -EINVAL;
1149+
if (vc->vc_cols > VC_MAXCOL || vc->vc_rows > VC_MAXROW ||
1150+
vc->vc_screenbuf_size > KMALLOC_MAX_SIZE || !vc->vc_screenbuf_size)
1151+
goto err_free;
1152+
err = -ENOMEM;
11391153
vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL);
11401154
if (!vc->vc_screenbuf)
11411155
goto err_free;
@@ -1154,7 +1168,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
11541168
visual_deinit(vc);
11551169
kfree(vc);
11561170
vc_cons[currcons].d = NULL;
1157-
return -ENOMEM;
1171+
return err;
11581172
}
11591173

11601174
static inline int resize_screen(struct vc_data *vc, int width, int height,
@@ -1169,14 +1183,6 @@ static inline int resize_screen(struct vc_data *vc, int width, int height,
11691183
return err;
11701184
}
11711185

1172-
/*
1173-
* Change # of rows and columns (0 means unchanged/the size of fg_console)
1174-
* [this is to be used together with some user program
1175-
* like resize that changes the hardware videomode]
1176-
*/
1177-
#define VC_RESIZE_MAXCOL (32767)
1178-
#define VC_RESIZE_MAXROW (32767)
1179-
11801186
/**
11811187
* vc_do_resize - resizing method for the tty
11821188
* @tty: tty being resized
@@ -1212,7 +1218,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
12121218
user = vc->vc_resize_user;
12131219
vc->vc_resize_user = 0;
12141220

1215-
if (cols > VC_RESIZE_MAXCOL || lines > VC_RESIZE_MAXROW)
1221+
if (cols > VC_MAXCOL || lines > VC_MAXROW)
12161222
return -EINVAL;
12171223

12181224
new_cols = (cols ? cols : vc->vc_cols);
@@ -1223,7 +1229,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
12231229
if (new_cols == vc->vc_cols && new_rows == vc->vc_rows)
12241230
return 0;
12251231

1226-
if (new_screen_size > KMALLOC_MAX_SIZE)
1232+
if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size)
12271233
return -EINVAL;
12281234
newscreen = kzalloc(new_screen_size, GFP_USER);
12291235
if (!newscreen)
@@ -3418,6 +3424,7 @@ static int __init con_init(void)
34183424
INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
34193425
tty_port_init(&vc->port);
34203426
visual_init(vc, currcons, 1);
3427+
/* Assuming vc->vc_{cols,rows,screenbuf_size} are sane here. */
34213428
vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
34223429
vc_init(vc, vc->vc_rows, vc->vc_cols,
34233430
currcons || !vc->vc_sw->con_save_screen, KD_TEXT);

0 commit comments

Comments
 (0)