Re: [PATCH v6 2/4] fbmem: Prevent invalid virtual screen sizes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Helge,

On Sun, Jun 26, 2022 at 12:32 PM Helge Deller <deller@xxxxxx> wrote:
> Prevent that drivers or the user sets the virtual screen resolution
> smaller than the physical screen resolution.  This is important, because
> otherwise we may access memory outside of the graphics memory area.
>
> Signed-off-by: Helge Deller <deller@xxxxxx>
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v5.4+

Thanks for your patch, which is now commit fe04405ce5de13a5 ("fbmem:
Prevent invalid virtual screen sizes") in fbdev/for-next.

> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1006,6 +1006,12 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
>         if (var->xres < 8 || var->yres < 8)
>                 return -EINVAL;
>
> +       /* make sure virtual resolution >= physical resolution */
> +       if (var->xres_virtual < var->xres)
> +               return -EINVAL;
> +       if (var->yres_virtual < var->yres)
> +               return -EINVAL;

This breaks valid use cases (e.g. "fbset -xres <larger-value-than-before>") ,
as the FBIOPUT_VSCREENINFO rule is to round up invalid values,
if possible.

Individual drivers may not follow that rule, so you could indeed end up
with a virtual resolution here if such a driver fails to sanitize var.
So either you have to move this after the call to fbops.fb_check_var()
below, and/or change the code to enlarge virtual resolution to match
physical resolution (at the risk of introducing another regression
with an obscure driver?).

So I'd go for moving it below.  And perhaps add a WARN(), as this
is a driver bug?

> +
>         /* Too huge resolution causes multiplication overflow. */
>         if (check_mul_overflow(var->xres, var->yres, &unused) ||
>             check_mul_overflow(var->xres_virtual, var->yres_virtual, &unused))

Note that doing the multiplication overflow check before calling
fbops.fb_check_var() is fine, as too large values can never be
rounded up to a valid value.

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



[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux