Hi Helge, On Thu, Jun 30, 2022 at 10:10 PM Helge Deller <deller@xxxxxx> wrote: > On 6/30/22 22:00, Geert Uytterhoeven wrote: > > On Thu, Jun 30, 2022 at 9:46 PM Helge Deller <deller@xxxxxx> wrote: > >> On 6/30/22 21:36, Geert Uytterhoeven wrote: > >>> On Thu, Jun 30, 2022 at 9:31 PM Helge Deller <deller@xxxxxx> wrote: > >>>> On 6/30/22 21:00, Geert Uytterhoeven wrote: > >>>>> On Wed, Jun 29, 2022 at 10:00 PM Helge Deller <deller@xxxxxx> wrote: > >>>>>> The virtual screen size can't be smaller than the physical screen size. > >>>>>> Based on the general rule that we round up user-provided input if > >>>>>> neccessary, adjust the virtual screen size as well if needed. > >>>>>> > >>>>>> Signed-off-by: Helge Deller <deller@xxxxxx> > >>>>>> Cc: stable@xxxxxxxxxxxxxxx # v5.4+ > >>>>> > >>>>> Thanks for your patch! > >>>>> > >>>>>> --- a/drivers/video/fbdev/core/fbmem.c > >>>>>> +++ b/drivers/video/fbdev/core/fbmem.c > >>>>>> @@ -1106,6 +1106,11 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, > >>>>>> return -EFAULT; > >>>>>> console_lock(); > >>>>>> lock_fb_info(info); > >>>>>> + /* adjust virtual screen size if user missed it */ > >>>>>> + if (var.xres_virtual < var.xres) > >>>>>> + var.xres_virtual = var.xres; > >>>>>> + if (var.yres_virtual < var.yres) > >>>>>> + var.yres_virtual = var.yres; > >>>>>> ret = fb_set_var(info, &var); > >>>>>> if (!ret) > >>>>>> fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL); > >>>>> > >>>>> Given "[PATCH 4/5] fbmem: Prevent invalid virtual screen sizes in > >>>>> fb_set_var", I don't think we need this patch. > >>>> > >>>> We do. > >>> > >>> Why? It will be caught by [PATCH 4/5]. > >> > >> Right, it will be caught by patch #4. > >> But if you drop this part, then everytime a user runs > >> fbset -xres 800 -yres 600 -xvres 200 > >> users will get the KERNEL BUG WARNING (from patch #4) including > >> a kernel backtrace in their syslogs. > > > > No, they will only see that warning if they are using a broken fbdev > > driver that implements .fb_check_var(), but fails to validate or > > update the passed geometry. > > IMHO this argument is mood. > That way you put pressure on and need such simple code in > each single driver to fix it up, instead of cleaning it up at a central > place. Most hardware has restrictions on resolution (e.g. xres must be a multiple of N), so the driver has to round up the resolution to make it fit. And after that the driver has to validate and update the virtual resolution again anyway... If a driver does not support changing the video mode, it can leave out the .fb_check_var() and .fb_set_par() callbacks, so the fbdev core will ignore the userspace-supplied parameters, and reinstate the single supported mode. See e.g. "[PATCH] drm/fb-helper: Remove helpers to change frame buffer config" (https://lore.kernel.org/all/20220629105658.1373770-1-geert@xxxxxxxxxxxxxx). 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
