On 6/25/22 14:45, Daniel Vetter wrote:
> On Sat, Jun 25, 2022 at 02:24:59PM +0200, Helge Deller 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 <[email protected]>
>> Cc: [email protected] # v4.14+
>> ---
>>  drivers/video/fbdev/core/fbcon.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/core/fbcon.c 
>> b/drivers/video/fbdev/core/fbcon.c
>> index c4e91715ef00..e162d5e753e5 100644
>> --- 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))
>> +            return -EINVAL;
>
> Reviewed-by: Daniel Vetter <[email protected]>

Thanks!

> Maybe as a safety follow up patch, we have a few copies of the below:
>
>       cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres);
>       rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
>       cols /= vc->vc_font.width;
>       rows /= vc->vc_font.height;
>
> Might be good to extract that into a helper and also add
>
>       WARN_ON(!cols);
>       WARN_ON(!rows);

That's not needed then.
The checks I added above will ensure that cols and rows are both greater than 0.

> to make sure we really didn't screw this up and give syzkaller et all an
> easier time finding bugs - it doesn't need to discover the full exploit,
> only needs to get to this here.
>
> Also maybe even check that cols/rows is within reasons, like smaller than
> BIT(24) or so (so that we have a bunch of headroom for overflows).

Not needed either.
cols and rows is the screen size divided by an value between 1-32 (the max
font size). So, since screen size is already the higest limit, cols&rows
will always be smaller than screen size (and > 0).

Helge

Reply via email to