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

[Helge Deller <deller@gmx.de>]

On 6/29/22 09:03, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Tue, Jun 28, 2022 at 10:52 PM Helge Deller <deller@gmx.de> wrote:
>> On 6/28/22 10:39, Geert Uytterhoeven wrote:
>>> 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/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.
>
> Isn't the font a per-VC setting? Hence can't you change resolution,
> switch to a different VC, and run into this?
>
> I think the only real solution is to set the number of text columns
> and/or rows to zero, and make sure that is handled correctly.

I agree, there doesn't seem to be a simple solution.
On the other hand, such usecase seems very unlikely.
If you have a proposal for a pacth I'd welcome it.

Anyway, I've just sent out a new patch series. It does not include any patch
for this theoretical problem yet.

Helge