On 6/30/22 21:09, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Wed, Jun 29, 2022 at 10:00 PM Helge Deller <deller@xxxxxx> 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@xxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx # 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@xxxxxxxxxxxxxx
>
> 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
