Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2)

From: Helge Deller <deller@gmx.de>
To: Khalid Masum <khalid.masum.92@gmail.com>,
	syzbot <syzbot+14b0e8f3fd1612e35350@syzkaller.appspotmail.com>,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2)
Date: Sat, 30 Jul 2022 19:25:54 +0200	[thread overview]
Message-ID: <eb4a26aa-da30-ceee-7d27-c1e902dd4218@gmx.de> (raw)
In-Reply-To: <20220729065139.6529-1-khalid.masum.92@gmail.com>

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:
ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3  ys:0  xe:0 ye:0 mode:0)  = 0
-> sets the text selection area
ioctl(4, KDFONTOP)  with op=0 (con_font_set), charcount=512  width=8  height=32, 0x20000000) = 0
-> changes the font size.

It does not crash with current Linus' head (v5.19-rc8).
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):

 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.

Helge