linux-kernel - Re: [PATCH] video: fbdev: vga16fb: fix OOB write in vga16fb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2105142150460.3032@angie.orcam.me.uk>
Date:   Fri, 14 May 2021 22:25:53 +0200 (CEST)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
cc:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Daniel Vetter <daniel@...ll.ch>,
        syzbot <syzbot+1f29e126cf461c4de3b3@...kaller.appspotmail.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Colin King <colin.king@...onical.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jani Nikula <jani.nikula@...el.com>,
        Jiri Slaby <jirislaby@...nel.org>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        "Antonino A. Daplas" <adaplas@...il.com>
Subject: Re: [PATCH] video: fbdev: vga16fb: fix OOB write in
 vga16fb_imageblit()

On Fri, 14 May 2021, Linus Torvalds wrote:

> > Currently it is impossible to control upper limit of rows/columns values
> > based on amount of memory reserved for the graphical screen, for
> > resize_screen() calls vc->vc_sw->con_resize() only if vc->vc_mode is not
> > already KD_GRAPHICS
> 
> Honestly, the saner approach would seem to be to simply error out if
> vc_mode is KD_GRAPHICS.
> 
> Doing VT_RESIZE while in KD_GRAPHICS mode seems _very_ questionable,
> and is clearly currently very buggy.

 I haven't looked into it any further beyond tracking down (again, using 
the LMO tree) the originating change as the other fix took precedence.  It 
came with:

commit 094e0a9cdbdf1e11a28dd756a6cbd750b6303d10
Author: Ralf Baechle <ralf@...ux-mips.org>
Date:   Sun Jun 1 12:07:37 2003 +0000

    Merge with Linux 2.5.51

along with framebuffer console support:

+inline int resize_screen(int currcons, int width, int height)
+{
+	/* Resizes the resolution of the display adapater */
+	int err = 0;
+
+	if (vcmode != KD_GRAPHICS && sw->con_resize)
+		err = sw->con_resize(vc_cons[currcons].d, width, height);
+	return err;
+}
+

A handler for fbcon was added shortly afterwards with:

commit bab384bdbe279efd7acc2146ef13b0b0395b2a42
Author: Ralf Baechle <ralf@...ux-mips.org>
Date:   Tue Jun 3 17:04:10 2003 +0000

    Merge with Linux 2.5.59.

however vgacon didn't have a handler for it until commit 28254d439b8c 
("[PATCH] vga text console and stty cols/rows") two years later only.

 Overall I think it does make sense to resize the text console at any 
time, even if the visible console (VT) chosen is in the graphics mode, as 
my understanding (and experience at least with vgacon) is that resizing 
the console applies globally across all the VTs.  So the intent of the 
original change appears valid to me, and the choice not to reprogram the 
visible console and only store the settings for a future use if it's in 
the graphics mode correct.

 Which means any bug triggered here needs to be fixed elsewhere rather 
than by making the request fail.

 NB for fbcon the usual ioctl to resize the console is FBIOPUT_VSCREENINFO 
rather than VT_RESIZEX; fbset(8) uses it, and I actually experimented with 
it and a TGA-like (SFB+) framebuffer when at my lab last time, as Linux is 
kind enough to know how to fiddle with its clockchip.  It works just fine.

  Maciej