Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <eb4a26aa-da30-ceee-7d27-c1e902dd4218@gmx.de>
Date:   Sat, 30 Jul 2022 19:25:54 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Content-Language: en-US
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>
References: <000000000000bbdd0405d120c155@google.com>
 <20220729065139.6529-1-khalid.masum.92@gmail.com>
From:   Helge Deller <deller@gmx.de>
Subject: Re: [syzbot] KASAN: vmalloc-out-of-bounds Write in imageblit (2)
In-Reply-To: <20220729065139.6529-1-khalid.masum.92@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On 7/29/22 08:51, Khalid Masum wrote:
> Here is a simplified reproducer for the issue:
>
> https://gist.githubusercontent.com/Labnann/923d6b9b3a19848fc129637b839b8=
a55/raw/a68271fcc724569735fe27f80817e561b3ff629a/reproducer.c

The reproducer does this:
ioctl(3, TIOCLINUX, TIOCL_SETSEL, selection: xs:3  ys:0  xe:0 ye:0 mode:0)=
  =3D 0
-> sets the text selection area
ioctl(4, KDFONTOP)  with op=3D0 (con_font_set), charcount=3D512  width=3D8=
  height=3D32, 0x20000000) =3D 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 backpor=
ts
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 (inva=
lid)
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 -EINV=
AL if found.

But this bug isn't critical itself and is not the reason for the kernel cr=
ash.
Without the checks, the ioctl handler simply wraps the coordinate values a=
nd converts them
from:
input selection: xs:3  ys:0  xe:0   ye:0  mode:0    to the new:
vc_selection =3D   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 KD=
FONTOP 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 visib=
le screen
and this finally triggeres this backtrace, because vc_do_resize() calls cl=
ear_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 curre=
nt
selection still fits with the selected font, which gets tricky because som=
e
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