From: Daniel Vetter <daniel@ffwll.ch> To: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>, Dave Airlie <airlied@gmail.com>, Linus Torvalds <torvalds@linux-foundation.org> Cc: akpm@osdl.org, pmladek@suse.com, wangkefeng.wang@huawei.com, b.zolnierkie@samsung.com, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, sergey.senozhatsky@gmail.com Subject: Re: [v4] vgacon: Fix a UAF in vgacon_invert_region Date: Fri, 06 Mar 2020 10:38:46 +0000 [thread overview] Message-ID: <20200306103846.GS2363188@phenom.ffwll.local> (raw) In-Reply-To: <20200304022429.37738-1-zhangxiaoxu5@huawei.com> On Wed, Mar 04, 2020 at 10:24:29AM +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_size in vgacon driver, then > bad access. > Also, if set an larger screenbuf firstly, then set an more larger > screenbuf, when copy old_origin to new_origin, a bad access may happen. > > So, If the screen size larger than vga_vram, resize screen should be > failed. This alse fix CVE-2020-8649 and CVE-2020-8647. > > Fixes: 0aec4867dca14 ("[PATCH] SVGATextMode fix") > Reference: CVE-2020-8647 and CVE-2020-8649 > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> This seems to match the size computation used in other places in the vt and vgacon code. So from that pov Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> There's also no other "is this going to be bigger than the vga memory we have" checks anywhere, so makes sense to do that. There's a few copy helpers that limit the copy size to the max of vga_ram_size and vc->vc_screenbuf_size, but seems like not really done consistently. So seems to make sense, but really how did I end up getting listed as maintainer for vt stuff :-/ Anyway also needs Cc: stable@vger.kernel.org Linus, since this missed the -fixes pull from Dave maybe double check I'm not grossly wrong here and apply directly? Thanks, Daniel > --- > drivers/video/console/vgacon.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > index de7b8382aba9..998b0de1812f 100644 > --- a/drivers/video/console/vgacon.c > +++ b/drivers/video/console/vgacon.c > @@ -1316,6 +1316,9 @@ 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 << 1) * height > vga_vram_size) > + return -EINVAL; > + > if (width % 2 || width > screen_info.orig_video_cols || > height > (screen_info.orig_video_lines * vga_default_font_height)/ > c->vc_font.height) > -- > 2.17.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch> To: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>, Dave Airlie <airlied@gmail.com>, Linus Torvalds <torvalds@linux-foundation.org> Cc: akpm@osdl.org, pmladek@suse.com, wangkefeng.wang@huawei.com, b.zolnierkie@samsung.com, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, sergey.senozhatsky@gmail.com Subject: Re: [v4] vgacon: Fix a UAF in vgacon_invert_region Date: Fri, 6 Mar 2020 11:38:46 +0100 [thread overview] Message-ID: <20200306103846.GS2363188@phenom.ffwll.local> (raw) In-Reply-To: <20200304022429.37738-1-zhangxiaoxu5@huawei.com> On Wed, Mar 04, 2020 at 10:24:29AM +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_size in vgacon driver, then > bad access. > Also, if set an larger screenbuf firstly, then set an more larger > screenbuf, when copy old_origin to new_origin, a bad access may happen. > > So, If the screen size larger than vga_vram, resize screen should be > failed. This alse fix CVE-2020-8649 and CVE-2020-8647. > > Fixes: 0aec4867dca14 ("[PATCH] SVGATextMode fix") > Reference: CVE-2020-8647 and CVE-2020-8649 > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> This seems to match the size computation used in other places in the vt and vgacon code. So from that pov Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> There's also no other "is this going to be bigger than the vga memory we have" checks anywhere, so makes sense to do that. There's a few copy helpers that limit the copy size to the max of vga_ram_size and vc->vc_screenbuf_size, but seems like not really done consistently. So seems to make sense, but really how did I end up getting listed as maintainer for vt stuff :-/ Anyway also needs Cc: stable@vger.kernel.org Linus, since this missed the -fixes pull from Dave maybe double check I'm not grossly wrong here and apply directly? Thanks, Daniel > --- > drivers/video/console/vgacon.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > index de7b8382aba9..998b0de1812f 100644 > --- a/drivers/video/console/vgacon.c > +++ b/drivers/video/console/vgacon.c > @@ -1316,6 +1316,9 @@ 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 << 1) * height > vga_vram_size) > + return -EINVAL; > + > if (width % 2 || width > screen_info.orig_video_cols || > height > (screen_info.orig_video_lines * vga_default_font_height)/ > c->vc_font.height) > -- > 2.17.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-03-06 10:38 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-04 2:24 [v4] vgacon: Fix a UAF in vgacon_invert_region Zhang Xiaoxu 2020-03-04 2:24 ` Zhang Xiaoxu 2020-03-06 10:38 ` Daniel Vetter [this message] 2020-03-06 10:38 ` Daniel Vetter 2020-03-06 12:55 ` Linus Torvalds 2020-03-06 12:55 ` Linus Torvalds 2020-03-06 13:12 ` Daniel Vetter 2020-03-06 13:12 ` Daniel Vetter 2020-03-06 13:24 ` Linus Torvalds 2020-03-06 13:24 ` Linus Torvalds
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200306103846.GS2363188@phenom.ffwll.local \ --to=daniel@ffwll.ch \ --cc=airlied@gmail.com \ --cc=akpm@osdl.org \ --cc=b.zolnierkie@samsung.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-fbdev@vger.kernel.org \ --cc=pmladek@suse.com \ --cc=sergey.senozhatsky@gmail.com \ --cc=torvalds@linux-foundation.org \ --cc=wangkefeng.wang@huawei.com \ --cc=zhangxiaoxu5@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.