From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> 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: [PATCH] vgacon: Fix a UAF in vgacon_invert_region Date: Tue, 03 Mar 2020 13:59:40 +0000 [thread overview] Message-ID: <20200303135940.GS13686@intel.com> (raw) In-Reply-To: <20200303032036.40560-1-zhangxiaoxu5@huawei.com> 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 -- Ville Syrjälä Intel
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> 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: [PATCH] vgacon: Fix a UAF in vgacon_invert_region Date: Tue, 3 Mar 2020 15:59:40 +0200 [thread overview] Message-ID: <20200303135940.GS13686@intel.com> (raw) In-Reply-To: <20200303032036.40560-1-zhangxiaoxu5@huawei.com> 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 -- Ville Syrjälä Intel _______________________________________________ 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-03 13:59 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-03 3:20 [PATCH] vgacon: Fix a UAF in vgacon_invert_region Zhang Xiaoxu 2020-03-03 3:20 ` Zhang Xiaoxu 2020-03-03 13:59 ` Ville Syrjälä [this message] 2020-03-03 13:59 ` Ville Syrjälä 2020-03-03 14:30 ` zhangxiaoxu (A) 2020-03-03 14:30 ` zhangxiaoxu (A) 2020-03-03 14:46 ` Ville Syrjälä 2020-03-03 14:46 ` Ville Syrjälä 2020-03-04 1:31 ` zhangxiaoxu (A) 2020-03-04 1:31 ` zhangxiaoxu (A)
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=20200303135940.GS13686@intel.com \ --to=ville.syrjala@linux.intel.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=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.