[PATCH 3/5] fbcon: Prevent that screen size is smaller than font size

[Helge Deller]

On 6/30/22 21:09, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Wed, Jun 29, 2022 at 10:00 PM Helge Deller <deller at 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 at gmx.de>
>> Cc: stable at vger.kernel.org # v5.4+
>
> Thanks for your patch!
>
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -2736,6 +2736,34 @@ void fbcon_update_vcs(struct fb_info *info, bool all)
>>  }
>>  EXPORT_SYMBOL(fbcon_update_vcs);
>>
>> +/* let fbcon check if it supports a new screen resolution */
>> +int fbcon_modechange_possible(struct fb_info *info, struct fb_var_screeninfo *var)
>> +{
>> +       struct fbcon_ops *ops = info->fbcon_par;
>> +       struct vc_data *vc;
>> +       int i;
>> +
>> +       WARN_CONSOLE_UNLOCKED();
>> +
>> +       if (!ops || ops->currcon < 0)
>> +               return -EINVAL;
>
> So if the virtual console is _not_ used as a text console, we refuse
> mode changes?

Right, this needs to be dropped.

Helge


>> +
>> +       /* prevent setting a screen size which is smaller than font size */
>> +       for (i = first_fb_vc; i <= last_fb_vc; i++) {
>> +               vc = vc_cons[i].d;
>> +               if (!vc || vc->vc_mode != KD_TEXT ||
>> +                          registered_fb[con2fb_map[i]] != info)
>> +                       continue;
>> +
>> +               if (vc->vc_font.width  > FBCON_SWAP(var->rotate, var->xres, var->yres) ||
>> +                   vc->vc_font.height > FBCON_SWAP(var->rotate, var->yres, var->xres))
>> +                       return -EINVAL;
>> +       }
>
> IMHO this looks way too fragile, and we should fix the rendering code
> to handle cols == 0 || rows == 0 instead...

Long-term we might need both.
The code above will at least return EINVAL if users try it, while
the cols/rows=0 code just prevents rendering if it doesn't fit.

Helge

>
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL(fbcon_modechange_possible);
>> +
>>  int fbcon_mode_deleted(struct fb_info *info,
>>                        struct fb_videomode *mode)
>>  {
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index e645b96d2700..324f726739c4 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1111,7 +1111,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>                         var.xres_virtual = var.xres;
>>                 if (var.yres_virtual < var.yres)
>>                         var.yres_virtual = var.yres;
>> -               ret = fb_set_var(info, &var);
>> +               ret = fbcon_modechange_possible(info, &var);
>> +               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 at 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