Re: [PATCH 1/5] fbcon: Disallow setting font bigger than screen size

From: Helge Deller <deller@gmx.de>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 1/5] fbcon: Disallow setting font bigger than screen size
Date: Thu, 30 Jun 2022 21:26:08 +0200	[thread overview]
Message-ID: <2c7c0791-2815-5e52-8959-a4ae179206a5@gmx.de> (raw)
In-Reply-To: <CAMuHMdWW-oh_q7zE6riS8o+tgRmcOxxO_93-FVy9CVCcAOakhg@mail.gmail.com>

On 6/30/22 20:51, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Wed, Jun 29, 2022 at 10:00 PM Helge Deller <deller@gmx.de> wrote:
>> Prevent that users set a font size which is bigger than the physical screen.
>> It's unlikely this may happen (because screens are usually much larger than the
>> fonts and each font char is limited to 32x32 pixels), but it may happen on
>> smaller screens/LCD displays.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Thanks for your patch!
>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -2469,6 +2469,11 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
>>         if (charcount != 256 && charcount != 512)
>>                 return -EINVAL;
>>
>> +       /* font bigger than screen resolution ? */
>> +       if (font->width  > FBCON_SWAP(info->var.rotate, info->var.xres, info->var.yres) ||
>> +           font->height > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
>
> Note that we already have local vars w and h, albeit with the wrong
> signedness.

I don't like the "h" and "w" variables. Maybe something like "fh" for "font-heigth"
would have been better explaining which kind of "h" is meant.
I assume that's why the patch below didn't used it either.

That said, I'd like to keep it as is (at least for now).

Helge

>> +               return -EINVAL;
>> +
>>         /* Make sure drawing engine can handle the font */
>>         if (!(info->pixmap.blit_x & (1 << (font->width - 1))) ||
>>             !(info->pixmap.blit_y & (1 << (font->height - 1))))
>
> There were already more opportunities for using w and h before...
>
> 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