* [v2] vt: fix use after free in function "vc_do_resize"
@ 2020-03-02 11:28 Ye Bin
2020-03-03 9:49 ` Jiri Slaby
2020-03-06 8:59 ` Jiri Slaby
0 siblings, 2 replies; 3+ messages in thread
From: Ye Bin @ 2020-03-02 11:28 UTC (permalink / raw)
To: gregkh, jslaby; +Cc: linux-serial, linux-kernel, yebin10
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.
Reported-by: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/tty/vt/vt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 8fa059ec6cc8..1d7217bef678 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -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,
--
2.17.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [v2] vt: fix use after free in function "vc_do_resize"
2020-03-02 11:28 [v2] vt: fix use after free in function "vc_do_resize" Ye Bin
@ 2020-03-03 9:49 ` Jiri Slaby
2020-03-06 8:59 ` Jiri Slaby
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Slaby @ 2020-03-03 9:49 UTC (permalink / raw)
To: Ye Bin, gregkh; +Cc: linux-serial, linux-kernel
Hi,
On 02. 03. 20, 12:28, Ye Bin wrote:
> 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".
this is much better. So this is one of the bugzilla reports...
> 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.
I need to look into the code first. Note to myself, the line was added in:
commit 9fc2b2d0cf743008d8f6be6293278f4ef61f09f3
Author: Francisco Jerez <currojerez@riseup.net>
Date: Sun Aug 22 17:37:24 2010 +0200
vt: Fix console corruption on driver hand-over.
> Reported-by: Jiri Slaby <jslaby@suse.com>
You misunderstood me. syzbot, while reporting a bug, amends this to the
reports:
"""
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+SOMETHING@syzkaller.appspotmail.com
"""
It is not the case in this case, as this went through bugzilla, not
using syzbot.
So instead of the Reported-by, we should have:
References: https://bugzilla.kernel.org/show_bug.cgi?id=206359
here.
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> drivers/tty/vt/vt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 8fa059ec6cc8..1d7217bef678 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -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,
>
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [v2] vt: fix use after free in function "vc_do_resize"
2020-03-02 11:28 [v2] vt: fix use after free in function "vc_do_resize" Ye Bin
2020-03-03 9:49 ` Jiri Slaby
@ 2020-03-06 8:59 ` Jiri Slaby
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Slaby @ 2020-03-06 8:59 UTC (permalink / raw)
To: Ye Bin, gregkh; +Cc: linux-serial, linux-kernel
On 02. 03. 20, 12:28, Ye Bin wrote:
> 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".
So I suppose you cannot reproduce the problem using the reproducer
attached in the bug when this patch is applied?
> 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.
>
> Reported-by: Jiri Slaby <jslaby@suse.com>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> drivers/tty/vt/vt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 8fa059ec6cc8..1d7217bef678 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -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,
>
--
js
suse labs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-06 8:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-02 11:28 [v2] vt: fix use after free in function "vc_do_resize" Ye Bin
2020-03-03 9:49 ` Jiri Slaby
2020-03-06 8:59 ` Jiri Slaby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).