Re: [PATCH] drm/fb-helper: Make set_var validation stricter

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

 



Update Michel's address.

On Tue, 21 Jun 2022 at 11:23, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
>
> The drm fbdev emulation does not forward mode changes to the driver,
> and hence all changes where rejected in 865afb11949e ("drm/fb-helper:
> reject any changes to the fbdev").
>
> Unfortunately this resulted in bugs on multiple monitor systems with
> different resolutions. In that case the fbdev emulation code sizes the
> underlying framebuffer for the largest screen (which dictates
> x/yres_virtual), but adjust the fbdev x/yres to match the smallest
> resolution. The above mentioned patch failed to realize that, and
> errornously validated x/yres against the fb dimensions.
>
> This was fixed by just dropping the validation for too small sizes,
> which restored vt switching with 12ffed96d436 ("drm/fb-helper: Allow
> var->x/yres(_virtual) < fb->width/height again").
>
> But this also restored all kinds of validation issues and their
> fallout in the notoriously buggy fbcon code for too small sizes. Since
> no one is volunteering to really make fbcon and vc/vt fully robust
> against these math issues make sure this barn door is closed for good
> again.
>
> Since it's a bit tricky to remember the x/yres we picked across both
> the newer generic fbdev emulation and the older code with more driver
> involvement, we simply check that it doesn't change. This relies on
> drm_fb_helper_fill_var() having done things correctly, and nothing
> having trampled it yet.
>
> Note that this leaves all the other fbdev drivers out in the rain.
> Given that distros have finally started to move away from those
> completely for real I think that's good enough. The code it spaghetti
> enough that I do not feel confident to even review fixes for it.
>
> What might help fbdev is doing something similar to what was done in
> a49145acfb97 ("fbmem: add margin check to fb_check_caps()") and ensure
> x/yres_virtual aren't too small, for some value of "too small". Maybe
> checking that they're at least x/yres makes sense?
>
> Fixes: 12ffed96d436 ("drm/fb-helper: Allow var->x/yres(_virtual) < fb->width/height again")
> Cc: Michel Dänzer <michel.daenzer@xxxxxxx>
> Cc: Daniel Stone <daniels@xxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <mripard@xxxxxxxxxx>
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.11+
> Cc: Helge Deller <deller@xxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: openeuler-security@xxxxxxxxxxxxx
> Cc: guodaxing@xxxxxxxxxx
> Cc: Weigang (Jimmy) <weigang12@xxxxxxxxxx>
> Reported-by: Weigang (Jimmy) <weigang12@xxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
> Note: Weigang asked for this to stay under embargo until it's all
> review and tested.
> -Daniel
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 695997ae2a7c..5664a177a404 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1355,8 +1355,8 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
>          * to KMS, hence fail if different settings are requested.
>          */
>         if (var->bits_per_pixel > fb->format->cpp[0] * 8 ||
> -           var->xres > fb->width || var->yres > fb->height ||
> -           var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
> +           var->xres != info->var.xres || var->yres != info->var.yres ||
> +           var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
>                 drm_dbg_kms(dev, "fb requested width/height/bpp can't fit in current fb "
>                           "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
>                           var->xres, var->yres, var->bits_per_pixel,
> --
> 2.36.0
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux