Re: [PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size

[Helge Deller <deller@gmx.de>]

On 6/28/22 10:39, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Sun, Jun 26, 2022 at 12:33 PM Helge Deller <deller@gmx.de> wrote:
>> We need to prevent that users configure a screen size which is smaller than the
>> currently selected font size. Otherwise rendering chars on the screen will
>> access memory outside the graphics memory region.
>>
>> This patch adds a new function fbcon_modechange_possible() which
>> implements this check and which later may be extended with other checks
>> if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
>> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
>> for a too small screen size.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: stable@vger.kernel.org # v5.4+
>
> Thanks for your patch, which is now commit f0b6a66d33ca6e7e ("fbcon:
> Prevent that screen size is smaller than font size") in fbdev/for-next
>
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>                         return -EFAULT;
>>                 console_lock();
>>                 lock_fb_info(info);
>> -               ret = fb_set_var(info, &var);
>> +               ret = fbcon_modechange_possible(info, &var);
>
> Again, this should be done (if done at all) after the call to
> fb_ops.check_var(), as it breaks the FBIOPUT_VSCREENINFO rounding rule.
>
> What if the user just wants to display graphics, not text?

Yes, I need to go back to an older version here too and check that
the test is only run on text consoles.
That check was dropped, due feedback that you could switch
back from graphics (e.g. X11) to text console at any time....so the
check for text-only is not correct.

> Can't the text console be disabled instead?

I think the solution is to return failure if switching back to text mode isn't possible if
fonts are bigger than the screen resolution. That will be another patch.

Thanks!

Helge


>
>> +               if (!ret)
>> +                       ret = fb_set_var(info, &var);
>>                 if (!ret)
>>                         fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
>>                 unlock_fb_info(info);
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds