Skip to contentSkip to conversation

patch "vt: fix use after free in function "vc_do_resize"" added to tty-testing

PATCH [email protected] 2 participants 3 messages Mar 24, 2020

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

vt: fix use after free in function "vc_do_resize"

to my tty git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
in the tty-testing branch.

The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)

The patch will be merged to the tty-next branch sometime soon, after it passes testing, and the merge window is open.

If you have any questions about this process, please let me know.

From 313a7425f23320844169046d83d8996c98fd8b1d Mon Sep 17 00:00:00 2001
From: Ye Bin <[email protected]>
Date: Mon, 2 Mar 2020 19:28:56 +0800
Subject: vt: fix use after free in function "vc_do_resize"

Fix CVE-2020-8647 (https://nvd.nist.gov/vuln/detail/CVE-2020-8647),
detail description about this CVE is in bugzilla "https://bugzilla.kernel.org/show_bug.cgi?id=206359".

error information:

BUG: KASan: use after free in vc_do_resize+0x49e/0xb30 at addr ffff88000016b9c0
Read of size 2 by task syz-executor.3/24164
page:ffffea0000005ac0 count:0 mapcount:0 mapping:          (null) index:0x0
page flags: 0xfffff00000000()
page dumped because: kasan: bad access detected
CPU: 0 PID: 24164 Comm: syz-executor.3 Not tainted 3.10.0-862.14.2.1.x86_64+ #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
Call Trace:
 [<ffffffffb059f309>] dump_stack+0x1e/0x20
 [<ffffffffaf8af957>] kasan_report+0x577/0x950
 [<ffffffffaf8ae652>] __asan_load2+0x62/0x80
 [<ffffffffafe3728e>] vc_do_resize+0x49e/0xb30
 [<ffffffffafe3795c>] vc_resize+0x3c/0x60
 [<ffffffffafe1d80d>] vt_ioctl+0x16ed/0x2670
 [<ffffffffafe0089a>] tty_ioctl+0x46a/0x1a10
 [<ffffffffaf92db3d>] do_vfs_ioctl+0x5bd/0xc40
 [<ffffffffaf92e2f2>] SyS_ioctl+0x132/0x170
 [<ffffffffb05c9b1b>] system_call_fastpath+0x22/0x27

In function vc_do_resize:
......

if (vc->vc_y > new_rows) {
	.......
	old_origin += first_copied_row * old_row_size;
} else
	first_copied_row = 0;
end = old_origin + old_row_size * min(old_rows, new_rows);
......
while (old_origin < end) {
	scr_memcpyw((unsigned short *) new_origin,
		    (unsigned short *) old_origin, rlth);
	if (rrem)
		scr_memsetw((void *)(new_origin + rlth),
			    vc->vc_video_erase_char, rrem);
	old_origin += old_row_size;
	new_origin += new_row_size;
}

......

We can see that before calculate variable "end" may update variable "old_origin" with "old_origin += first_copied_row * old_row_size", variable "end" is equal to "old_origin + (first_copied_row + min(old_rows, new_rows))* old_row_size", it's possible that "first_copied_row + min(old_rows, new_rows)" large than "old_rows". So when call scr_memcpyw function cpoy data from origin buffer to new buffer in "while" loop, which "old_origin" may large than real old buffer end. Now, we calculate origin buffer end before update "old_origin" to avoid illegal memory access.

Cc Jiri Slaby <[email protected]>
Cc stable <[email protected]>

References: https://bugzilla.kernel.org/show_bug.cgi?id=206359

---
 drivers/tty/vt/vt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/vt/vt.c +1 -1
index bbc26d73209a..60e60611141a 100644
@@ -1231,6 +1231,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	old_origin = vc->vc_origin;
 	new_origin = (long) newscreen;
 	new_scr_end = new_origin + new_screen_size;
+	end = old_origin + old_row_size * min(old_rows, new_rows);
 
 	if (vc->vc_y > new_rows) {
 		if (old_rows - vc->vc_y < new_rows) {
@@ -1249,7 +1250,6 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 		old_origin += first_copied_row * old_row_size;
 	} else
 		first_copied_row = 0;
-	end = old_origin + old_row_size * min(old_rows, new_rows);
 
 	vc_uniscr_copy_area(new_uniscr, new_cols, new_rows,
 			    get_vc_uniscr(vc), rlth/2, first_copied_row,
Signature · 1 line
2.25.2
#1 lore raw
Quoted 4 lines · On 24. 03. 20, 12:31, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

vt: fix use after free in function "vc_do_resize"

I lost track about this one, but isn't this the patch which was withdrawn in favor of another patch really fixing the problem?

Quoted 104 lines

to my tty git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
in the tty-testing branch.

The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)

The patch will be merged to the tty-next branch sometime soon, after it passes testing, and the merge window is open.

If you have any questions about this process, please let me know.

From 313a7425f23320844169046d83d8996c98fd8b1d Mon Sep 17 00:00:00 2001
From: Ye Bin <[email protected]>
Date: Mon, 2 Mar 2020 19:28:56 +0800
Subject: vt: fix use after free in function "vc_do_resize"

Fix CVE-2020-8647 (https://nvd.nist.gov/vuln/detail/CVE-2020-8647),
detail description about this CVE is in bugzilla "https://bugzilla.kernel.org/show_bug.cgi?id=206359".

error information:

BUG: KASan: use after free in vc_do_resize+0x49e/0xb30 at addr ffff88000016b9c0
Read of size 2 by task syz-executor.3/24164
page:ffffea0000005ac0 count:0 mapcount:0 mapping:          (null) index:0x0
page flags: 0xfffff00000000()
page dumped because: kasan: bad access detected
CPU: 0 PID: 24164 Comm: syz-executor.3 Not tainted 3.10.0-862.14.2.1.x86_64+ #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
Call Trace:
 [<ffffffffb059f309>] dump_stack+0x1e/0x20
 [<ffffffffaf8af957>] kasan_report+0x577/0x950
 [<ffffffffaf8ae652>] __asan_load2+0x62/0x80
 [<ffffffffafe3728e>] vc_do_resize+0x49e/0xb30
 [<ffffffffafe3795c>] vc_resize+0x3c/0x60
 [<ffffffffafe1d80d>] vt_ioctl+0x16ed/0x2670
 [<ffffffffafe0089a>] tty_ioctl+0x46a/0x1a10
 [<ffffffffaf92db3d>] do_vfs_ioctl+0x5bd/0xc40
 [<ffffffffaf92e2f2>] SyS_ioctl+0x132/0x170
 [<ffffffffb05c9b1b>] system_call_fastpath+0x22/0x27

In function vc_do_resize:
......

if (vc->vc_y > new_rows) {
	.......
	old_origin += first_copied_row * old_row_size;
} else
	first_copied_row = 0;
end = old_origin + old_row_size * min(old_rows, new_rows);
......
while (old_origin < end) {
	scr_memcpyw((unsigned short *) new_origin,
		    (unsigned short *) old_origin, rlth);
	if (rrem)
		scr_memsetw((void *)(new_origin + rlth),
			    vc->vc_video_erase_char, rrem);
	old_origin += old_row_size;
	new_origin += new_row_size;
}

......

We can see that before calculate variable "end" may update variable "old_origin" with "old_origin += first_copied_row * old_row_size", variable "end" is equal to "old_origin + (first_copied_row + min(old_rows, new_rows))* old_row_size", it's possible that "first_copied_row + min(old_rows, new_rows)" large than "old_rows". So when call scr_memcpyw function cpoy data from origin buffer to new buffer in "while" loop, which "old_origin" may large than real old buffer end. Now, we calculate origin buffer end before update "old_origin" to avoid illegal memory access.

Cc Jiri Slaby <[email protected]>
Cc stable <[email protected]>

References: https://bugzilla.kernel.org/show_bug.cgi?id=206359

---
 drivers/tty/vt/vt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/vt/vt.c +1 -1
index bbc26d73209a..60e60611141a 100644
@@ -1231,6 +1231,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 	old_origin = vc->vc_origin;
 	new_origin = (long) newscreen;
 	new_scr_end = new_origin + new_screen_size;
+	end = old_origin + old_row_size * min(old_rows, new_rows);
 
 	if (vc->vc_y > new_rows) {
 		if (old_rows - vc->vc_y < new_rows) {
@@ -1249,7 +1250,6 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
 		old_origin += first_copied_row * old_row_size;
 	} else
 		first_copied_row = 0;
-	end = old_origin + old_row_size * min(old_rows, new_rows);
 
 	vc_uniscr_copy_area(new_uniscr, new_cols, new_rows,
 			    get_vc_uniscr(vc), rlth/2, first_copied_row,
Signature · 2 lines
js
suse labs
author maintainer
#2 lore raw
Quoted 8 lines · Jiri Slaby
Quoted 4 lines · On 24. 03. 20, 12:31, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

vt: fix use after free in function "vc_do_resize"

I lost track about this one, but isn't this the patch which was withdrawn in favor of another patch really fixing the problem?

Was it? I didn't think so. But I have no idea. I'll go drop this now and wait for confirmation from Ye.

thanks,

greg k-h