When syzkaller tests, there is a UAF: BUG: KASan: use after free in vgacon_invert_region+0x9d/0x110 at addr ffff880000100000 Read of size 2 by task syz-executor.1/16489 page:ffffea0000004000 count:0 mapcount:-127 mapping: (null) index:0x0 page flags: 0xfffff00000000() page dumped because: kasan: bad access detected CPU: 1 PID: 16489 Comm: syz-executor.1 Not tainted Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014 Call Trace: [<ffffffffb119f309>] dump_stack+0x1e/0x20 [<ffffffffb04af957>] kasan_report+0x577/0x950 [<ffffffffb04ae652>] __asan_load2+0x62/0x80 [<ffffffffb090f26d>] vgacon_invert_region+0x9d/0x110 [<ffffffffb0a39d95>] invert_screen+0xe5/0x470 [<ffffffffb0a21dcb>] set_selection+0x44b/0x12f0 [<ffffffffb0a3bfae>] tioclinux+0xee/0x490 [<ffffffffb0a1d114>] vt_ioctl+0xff4/0x2670 [<ffffffffb0a0089a>] tty_ioctl+0x46a/0x1a10 [<ffffffffb052db3d>] do_vfs_ioctl+0x5bd/0xc40 [<ffffffffb052e2f2>] SyS_ioctl+0x132/0x170 [<ffffffffb11c9b1b>] system_call_fastpath+0x22/0x27 Memory state around the buggy address: ffff8800000fff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffff8800000fff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >ffff880000100000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
It can be reproduce in the linux mainline by the program: #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> #include <sys/ioctl.h> #include <linux/vt.h>
struct tiocl_selection { unsigned short xs; /* X start */ unsigned short ys; /* Y start */ unsigned short xe; /* X end */ unsigned short ye; /* Y end */ unsigned short sel_mode; /* selection mode */ };
#define TIOCL_SETSEL 2 struct tiocl { unsigned char type; unsigned char pad; struct tiocl_selection sel; };
int main() { int fd = 0; const char *dev = "/dev/char/4:1";
struct vt_consize v = {0}; struct tiocl tioc = {0};
fd = open(dev, O_RDWR, 0);
v.v_rows = 3346; ioctl(fd, VT_RESIZEX, &v);
tioc.type = TIOCL_SETSEL; ioctl(fd, TIOCLINUX, &tioc);
return 0; }
When resize the screen, update the 'vc->vc_size_row' to the new_row_size, but when 'set_origin' in 'vgacon_set_origin', vgacon use 'vga_vram_base' for 'vc_origin' and 'vc_visible_origin', not 'vc_screenbuf'. It maybe smaller than 'vc_screenbuf'. When TIOCLINUX, use the new_row_size to calc the offset, it maybe larger than the vga_vram_base in vgacon driver, then bad access.
So, If the screen size larger than vga_vram, resize screen should be failed. This alse fix CVE-2020-8649
Fixes: 0aec4867dca14 ("[PATCH] SVGATextMode fix") Reported-by: Hulk Robot hulkci@huawei.com Signed-off-by: Zhang Xiaoxu zhangxiaoxu5@huawei.com --- drivers/video/console/vgacon.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c index de7b8382aba9..9c216f707629 100644 --- a/drivers/video/console/vgacon.c +++ b/drivers/video/console/vgacon.c @@ -1316,7 +1316,10 @@ static int vgacon_font_get(struct vc_data *c, struct console_font *font) static int vgacon_resize(struct vc_data *c, unsigned int width, unsigned int height, unsigned int user) { - if (width % 2 || width > screen_info.orig_video_cols || + if (width % 2 || width * height > vga_vram_size) + return -EINVAL; + + if (width > screen_info.orig_video_cols || height > (screen_info.orig_video_lines * vga_default_font_height)/ c->vc_font.height) /* let svgatextmode tinker with video timings and
On Tue, Mar 03, 2020 at 11:20:36AM +0800, Zhang Xiaoxu wrote:
When syzkaller tests, there is a UAF: BUG: KASan: use after free in vgacon_invert_region+0x9d/0x110 at addr ffff880000100000 Read of size 2 by task syz-executor.1/16489 page:ffffea0000004000 count:0 mapcount:-127 mapping: (null) index:0x0 page flags: 0xfffff00000000() page dumped because: kasan: bad access detected CPU: 1 PID: 16489 Comm: syz-executor.1 Not tainted Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014 Call Trace: [<ffffffffb119f309>] dump_stack+0x1e/0x20 [<ffffffffb04af957>] kasan_report+0x577/0x950 [<ffffffffb04ae652>] __asan_load2+0x62/0x80 [<ffffffffb090f26d>] vgacon_invert_region+0x9d/0x110 [<ffffffffb0a39d95>] invert_screen+0xe5/0x470 [<ffffffffb0a21dcb>] set_selection+0x44b/0x12f0 [<ffffffffb0a3bfae>] tioclinux+0xee/0x490 [<ffffffffb0a1d114>] vt_ioctl+0xff4/0x2670 [<ffffffffb0a0089a>] tty_ioctl+0x46a/0x1a10 [<ffffffffb052db3d>] do_vfs_ioctl+0x5bd/0xc40 [<ffffffffb052e2f2>] SyS_ioctl+0x132/0x170 [<ffffffffb11c9b1b>] system_call_fastpath+0x22/0x27 Memory state around the buggy address: ffff8800000fff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffff8800000fff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >ffff880000100000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
It can be reproduce in the linux mainline by the program: #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> #include <sys/ioctl.h> #include <linux/vt.h>
struct tiocl_selection { unsigned short xs; /* X start */ unsigned short ys; /* Y start */ unsigned short xe; /* X end */ unsigned short ye; /* Y end */ unsigned short sel_mode; /* selection mode */ };
#define TIOCL_SETSEL 2 struct tiocl { unsigned char type; unsigned char pad; struct tiocl_selection sel; };
int main() { int fd = 0; const char *dev = "/dev/char/4:1";
struct vt_consize v = {0}; struct tiocl tioc = {0}; fd = open(dev, O_RDWR, 0); v.v_rows = 3346; ioctl(fd, VT_RESIZEX, &v); tioc.type = TIOCL_SETSEL; ioctl(fd, TIOCLINUX, &tioc); return 0;}
When resize the screen, update the 'vc->vc_size_row' to the new_row_size, but when 'set_origin' in 'vgacon_set_origin', vgacon use 'vga_vram_base' for 'vc_origin' and 'vc_visible_origin', not 'vc_screenbuf'. It maybe smaller than 'vc_screenbuf'. When TIOCLINUX, use the new_row_size to calc the offset, it maybe larger than the vga_vram_base in vgacon driver, then bad access.
So, If the screen size larger than vga_vram, resize screen should be failed. This alse fix CVE-2020-8649
Fixes: 0aec4867dca14 ("[PATCH] SVGATextMode fix") Reported-by: Hulk Robot hulkci@huawei.com Signed-off-by: Zhang Xiaoxu zhangxiaoxu5@huawei.com
drivers/video/console/vgacon.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c index de7b8382aba9..9c216f707629 100644 --- a/drivers/video/console/vgacon.c +++ b/drivers/video/console/vgacon.c @@ -1316,7 +1316,10 @@ static int vgacon_font_get(struct vc_data *c, struct console_font *font) static int vgacon_resize(struct vc_data *c, unsigned int width, unsigned int height, unsigned int user) {
- if (width % 2 || width > screen_info.orig_video_cols ||
- if (width % 2 || width * height > vga_vram_size)
That doesn't match how vc_screenbuf_size is computed elsewhere. Also a lot of places seem to assume that the screenbuf can be larger than vga_vram_size (eg. all the memcpy()s pick the smaller size of the two).
And you're changing the behaviour of the code when 'width % 2 && user' is true.
return -EINVAL;- if (width > screen_info.orig_video_cols || height > (screen_info.orig_video_lines * vga_default_font_height)/ c->vc_font.height) /* let svgatextmode tinker with video timings and
-- 2.17.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
在 2020/3/3 21:59, Ville Syrjälä 写道:
That doesn't match how vc_screenbuf_size is computed elsewhere. Also a lot of places seem to assume that the screenbuf can be larger than vga_vram_size (eg. all the memcpy()s pick the smaller size of the two).
Yes, in the vga source code, we also pick the smaller size of two. But in other place, eg: vc_do_resize, copy the old_origin to new_origin, we not do that. It also make bad access happen. it maybe CVE-2020-8647.
I think we should just assume the width/height maybe larger than the default, not the screenbuf larger than vga_vram_size.
If not, any useful of the larger screenbuf?
And you're changing the behaviour of the code when 'width % 2 && user' is true
On Tue, Mar 03, 2020 at 10:30:14PM +0800, zhangxiaoxu (A) wrote:
在 2020/3/3 21:59, Ville Syrjälä 写道:
That doesn't match how vc_screenbuf_size is computed elsewhere. Also a lot of places seem to assume that the screenbuf can be larger than vga_vram_size (eg. all the memcpy()s pick the smaller size of the two).
Yes, in the vga source code, we also pick the smaller size of two. But in other place, eg: vc_do_resize, copy the old_origin to new_origin, we not do that. It also make bad access happen. it maybe CVE-2020-8647.
I think we should just assume the width/height maybe larger than the default, not the screenbuf larger than vga_vram_size.
If not, any useful of the larger screenbuf?
Maybe used for scrolling?
And you're changing the behaviour of the code when 'width % 2 && user' is true
在 2020/3/3 22:46, Ville Syrjälä 写道:
On Tue, Mar 03, 2020 at 10:30:14PM +0800, zhangxiaoxu (A) wrote:
在 2020/3/3 21:59, Ville Syrjälä 写道:
That doesn't match how vc_screenbuf_size is computed elsewhere. Also a lot of places seem to assume that the screenbuf can be larger than vga_vram_size (eg. all the memcpy()s pick the smaller size of the two).
Yes, in the vga source code, we also pick the smaller size of two. But in other place, eg: vc_do_resize, copy the old_origin to new_origin, we not do that. It also make bad access happen. it maybe CVE-2020-8647.
I think we should just assume the width/height maybe larger than the default, not the screenbuf larger than vga_vram_size.
If not, any useful of the larger screenbuf?
Maybe used for scrolling?
The screenbuf just allocated with cols and rows, it can be save just one screen? vc_do_resize is the largest size which one screen can be shown?
If so, we can't set the screen to the resolution which more than it's capability?
And you're changing the behaviour of the code when 'width % 2 && user' is true
dri-devel@lists.freedesktop.org
-
Ville Syrjälä -
Zhang Xiaoxu -
zhangxiaoxu (A)