On 6/25/22 15:00, Daniel Vetter wrote:
> On Sat, Jun 25, 2022 at 02:56:42PM +0200, Daniel Vetter wrote:
>> On Sat, Jun 25, 2022 at 02:25:01PM +0200, Helge Deller wrote:
>>> Enhance the checks in the FBIOPUT_VSCREENINFO ioctl handler to verify
>>> the user-provided new screen size for:
>>>
>>> a) virtual screen size >= physical screen size, and
>>>
>>> b) new screen size is bigger than currently configured console font size.
>>>
>>> Return -EINVAL on invalid input.
>>>
>>> Signed-off-by: Helge Deller <deller@xxxxxx>
>>> Cc: stable@xxxxxxxxxxxxxxx # v5.4+
>>
>> Imo squash this into the previous one please. Doesn't make sense to split
>> the patch which adds a function from it's callsite.
>
> Correction. The part to add the fbcon_modechange_possible call should be
> squashed into the previos patch.
Ok... based on my last mail, I then suggest to split that part out as another
follow-up patch. :-)
> The check for x/yres_virtaul < x/yres should imo be moved into fb_set_var,
> next to the other existing checks that have been added over time.
That was exactly the way I had coded it in the first round.
But you then suggested to move it to the ioctl code path...
I can easily change it back accordingly, but then we need to drop the
WARN_ON() [which means to drop PATCH 4] because otherwise you possibly
trigger the WARN_ON() if the user calls with wrong input values.
So, insted of
if (WARN_ON(var->xres_virtual < var->xres)) ...
it will become:
if (var->xres_virtual < var->xres) ...
I'll leave it up to you to decide...
Helge
> -Daniel
>
>> -Daniel
>>
>>> ---
>>> drivers/video/fbdev/core/fbmem.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>>> index afa2863670f3..50fb66b954d6 100644
>>> --- a/drivers/video/fbdev/core/fbmem.c
>>> +++ b/drivers/video/fbdev/core/fbmem.c
>>> @@ -1106,7 +1106,13 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>> return -EFAULT;
>>> console_lock();
>>> lock_fb_info(info);
>>> - ret = fb_set_var(info, &var);
>>> + if (var.xres_virtual < var.xres ||
>>> + var.yres_virtual < var.yres)
>>> + ret = -EINVAL;
>>> + if (!ret)
>>> + ret = fbcon_modechange_possible(info, &var);
>>> + if (!ret)
>>> + ret = fb_set_var(info, &var);
>>> if (!ret)
>>> fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
>>> unlock_fb_info(info);
>>> --
>>> 2.35.3
>>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
