Messages in this thread | | | Date | Sun, 31 Jul 2022 16:03:43 +0600 | Subject | Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2) | From | Khalid Masum <> |
| |
On 7/30/22 23:25, Helge Deller wrote: > On 7/29/22 08:51, Khalid Masum wrote: >> Here is a simplified reproducer for the issue: >> >> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c > > The reproducer does this:
Thanks for Looking into this. Being to this, so I have some questions. > ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3 ys:0 xe:0 ye:0 mode:0) = 0
How did you find out the selection values? From strace and man pages I know the third argument is an address.
> -> sets the text selection area > ioctl(4, KDFONTOP) with op=0 (con_font_set), charcount=512 width=8 height=32, 0x20000000) = 0
Same here, It would be very helpful if you could tell me how.
> -> changes the font size. > > It does not crash with current Linus' head (v5.19-rc8).
I tested in 5.19-rc8 in Qemu x86_64 and it crashed for me. > Kernel v5.16, which was used by this KASAN report, hasn't received backports > since months, so I tried stable kernel v5.15.58 instead, and this > kernel crashed with the reproducer. > > The reproducer brings up two issues with current code: > 1. The reproducer uses ioctl(TIOCLINUX, TIOCL_SETSEL) and hands over (invalid) > zero-values for ys and ye for the starting lines. > This is wrong, since the API seems to expect a "1" as the very first line for the selection. > This can be easily fixed by adding checks for zero-values and return -EINVAL if found. > > But this bug isn't critical itself and is not the reason for the kernel crash. > Without the checks, the ioctl handler simply wraps the coordinate values and converts them > from: > input selection: xs:3 ys:0 xe:0 ye:0 mode:0 to the new: > vc_selection = xs:2 ys:23 xe:127 ye:23 mode:0 > which is the current maximum coordinates for the screen. > > Those higher values now trigger issue #2: > After the TIOCL_SETSEL the last line on the screen is now selected. The KDFONTOP ioctl > then sets a 8x32 console font, and replaces the former 8x16 console font. > With the bigger font the current screen selection is now outside the visible screen > and this finally triggeres this backtrace, because vc_do_resize() calls clear_selection() > to unhighlight the selection (which starts to render chars outside of the screen):
That makes sense.
> drm_fb_helper_sys_imageblit drivers/gpu/drm/drm_fb_helper.c:794 [inline] > drm_fbdev_fb_imageblit+0x15c/0x350 drivers/gpu/drm/drm_fb_helper.c:2288 > bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:124 [inline] > bit_putcs+0x6e1/0xd20 drivers/video/fbdev/core/bitblit.c:173 > fbcon_putcs+0x353/0x440 drivers/video/fbdev/core/fbcon.c:1277 > do_update_region+0x399/0x630 drivers/tty/vt/vt.c:676 > invert_screen+0x1d4/0x600 drivers/tty/vt/vt.c:800 > highlight drivers/tty/vt/selection.c:57 [inline] > clear_selection drivers/tty/vt/selection.c:84 [inline] > clear_selection+0x55/0x70 drivers/tty/vt/selection.c:80 > vc_do_resize+0xe6e/0x1180 drivers/tty/vt/vt.c:1257 > > IMHO the easiest way to prevent this crash is to simply clear the > selection before the various con_font_set() console handlers are called. > Otherwise every console driver needs to add checks and verify if the current > selection still fits with the selected font, which gets tricky because some > of those drivers fiddle with the screen width&height before calling vc_do_resize(). > > I'll follow up to this mail with patches for both issues shortly.
I tested the patches. The crash no longer occurs with the reproducer.
> Helge
Thanks, -- Khalid Masum
| |