linux-kernel - Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT

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] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2104121433040.65251@angie.orcam.me.uk>
Date:   Mon, 12 Apr 2021 15:30:16 +0200 (CEST)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Daniel Vetter <daniel@...ll.ch>
cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        syzbot <syzbot+b308f5fd049fbbc6e74f@...kaller.appspotmail.com>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Greg KH <gregkh@...uxfoundation.org>,
        Helge Deller <deller@....de>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Martin Hostettler <textshell@...uujin.de>,
        George Kennedy <george.kennedy@...cle.com>,
        Jiri Slaby <jirislaby@...nel.org>,
        Peilin Ye <yepeilin.cs@...il.com>
Subject: Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE

On Mon, 12 Apr 2021, Daniel Vetter wrote:

> > Note that it's entirely possible that things continue to work well
> > despite this warning. It's unclear to me from your email if you
> > actually see any difference (and apparently you're not able to see it
> > right now due to not being close to the machine).
> 
> Original search didn't turn up any users of VT_RESIZEX, this is the
> first. And looking at the source code I think we could outright remove
> support for VT_RESIZEX (but make it silent) and everything should keep
> working:
> 
>         /*
>          * ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX
> on a 1.3.3 or higher kernel,
>          * until those kernel programmers make this unambiguous
>          */
> 
>        if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows,
> resize1x1)) sresize=TRUE;
> 
>        if (check_kernel_version(1,3,3, "VT_RESIZEX"))
>          {
>            /*
>             * VDisplay must de divided by 2 for DoubleScan modes,
>             * or VT_RESIZEX will fail -- until someone fixes the kernel
>             * so it understands about doublescan modes.
>             */
>            if (do_VT_RESIZEX(curr_textmode->cols,
>                              curr_textmode->rows,
>                              curr_textmode->VDisplay /
> (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
>                              curr_textmode->FontHeight,
>                              curr_textmode->HDisplay/8*curr_textmode->FontWidth,
>                              curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
>          }
> 
> The functions are just straightforward wrappers. There's also no cvs
> repo, changelog or old releases before 2000 that would shed some light
> on why this code even exists.

 I did some archaeology then, using a local copy of the linux-mips.org 
Linux tree that has historic information imported from the old oss.sgi.com 
MIPS/Linux CVS repo.  According to that the ioctl was added with or 
shortly before 2.1.14:

commit beb116954b9b7f3bb56412b2494b562f02b864b1
Author: Ralf Baechle <ralf@...ux-mips.org>
Date:   Tue Jan 7 02:33:00 1997 +0000

    Import of Linux/MIPS 2.1.14

and, importantly, it was used to set some parameters: 

		if ( vlin )
		  video_scan_lines = vlin;
		if ( clin )
		  video_font_height = clin;

used by `con_adjust_height' in drivers/char/vga.c: `video_scan_lines' to 
set the vertical display limit (so that it is a whole multiple of the new 
font height) and `video_font_height' to set the cursor scan lines in the 
CRTC.  The function was used by the PIO_FONTX and PIO_FONTRESET VT ioctls 
at that point.

 That code was moved to `vgacon_adjust_height' in drivers/video/vgacon.c 
and then drivers/video/console/vgacon.c.  The code is still there, serving 
the KDFONTOP ioctl.

 With:

commit 9736a3546de7b6a2b16ad93539e4b3ac72b385bb
Author: Ralf Baechle <ralf@...ux-mips.org>
Date:   Thu Jun 5 10:06:35 2003 +0000

    Merge with Linux 2.5.66.

the parameters were moved into `struct vc_data':

 		if (vlin)
-			video_scan_lines = vlin;
+			vc->vc_scan_lines = vlin;
 		if (clin)
-			video_font_height = clin;
+			vc->vc_font.height = clin;

and this piece of code to set them only removed with the change discussed 
here.

 So without even looking at the VT, which I'll surely get to eventually, I 
conclude this change regresses font resizing (KD_FONT_OP_SET) once a new 
resolution has been set with svgatextmode.  I think this change needs to 
be reverted, especially as the problematic PIO_FONT ioctl referred has 
been since removed with commit ff2047fb755d ("vt: drop old FONT ioctls").

  Maciej