Re: [PATCH 2/5] fbcon: Fix up user-provided virtual screen size

[Helge Deller <deller@gmx.de>]

On 6/30/22 21:36, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Thu, Jun 30, 2022 at 9:31 PM Helge Deller <deller@gmx.de> wrote:
>> On 6/30/22 21:00, Geert Uytterhoeven wrote:
>>> On Wed, Jun 29, 2022 at 10:00 PM Helge Deller <deller@gmx.de> 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@gmx.de>
>>>> Cc: stable@vger.kernel.org # 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.
This is not what you want.

If you drop the WARN() from patch #4 you'll end up with my
last patch series.



>>> Moreover, this
>>> patch will prevent triggering the WARN_ON()s in [PATCH 4/5]
>>
>> Right.
>>
>>> in the most common buggy case of drivers plainly ignoring var.[xy]res_virtual.
>>
>> In summary:
>> This patch #2 is fixing up user-space invalid input and is not
>> allowed to trigger any WARN_ON().
>
> It's the responsibility of the driver to at least look at its parameters.
> What other invalid values might it let pass, that we cannot catch
> at the generic level?
>
>> We could drop patch #4, but then we wouldn't catch bad drivers.
>
> I do want to keep patch #4.
>
> 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