linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	syzbot <syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com>,
	Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Greg KH <gregkh@linuxfoundation.org>,
	Helge Deller <deller@gmx.de>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Martin Hostettler <textshell@uchuujin.de>,
	George Kennedy <george.kennedy@oracle.com>,
	Jiri Slaby <jirislaby@kernel.org>,
	Peilin Ye <yepeilin.cs@gmail.com>
Subject: Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
Date: Mon, 12 Apr 2021 15:30:16 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2104121433040.65251@angie.orcam.me.uk> (raw)
In-Reply-To: <CAKMK7uH4+SGr0=FDBiTsMg+iE1ztiuP2QBxsgcvHNhd38ocndg@mail.gmail.com>

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@linux-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@linux-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

      reply	other threads:[~2021-04-12 13:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-23 17:30 KASAN: use-after-free Read in bit_putcs syzbot
2020-09-26  2:03 ` syzbot
2020-09-26 16:25   ` Tetsuo Handa
2020-09-26 19:39     ` Peilin Ye
2020-09-27  0:25     ` Tetsuo Handa
2020-09-27  8:28       ` Tetsuo Handa
2020-09-27  9:27         ` Peilin Ye
2020-09-27 11:46           ` [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE Tetsuo Handa
2020-09-27 12:06             ` Greg KH
2020-09-28 17:59             ` Martin Hostettler
2020-09-29  1:12               ` Tetsuo Handa
2020-09-29 10:52                 ` Martin Hostettler
2020-09-29 16:56                   ` Daniel Vetter
2020-09-29 17:10                     ` Greg KH
2021-04-11 21:43                       ` Maciej W. Rozycki
2021-04-11 22:15                         ` Linus Torvalds
2021-04-12  7:01                           ` Daniel Vetter
2021-04-12 13:30                             ` Maciej W. Rozycki [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.2104121433040.65251@angie.orcam.me.uk \
    --to=macro@orcam.me.uk \
    --cc=b.zolnierkie@samsung.com \
    --cc=daniel@ffwll.ch \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=george.kennedy@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=textshell@uchuujin.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yepeilin.cs@gmail.com \
    --subject='Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).