[PATCH] fbdev: Don't spam dmesg on bad userspace ioctl input

[Michel Dänzer]

On 4/11/23 11:10, Geert Uytterhoeven wrote:
> Hi Michel,
> 
> On Wed, Apr 5, 2023 at 10:50 AM Michel Dänzer
> <michel.daenzer at mailbox.org> wrote:
>> On 4/4/23 14:36, Daniel Vetter wrote:
>>> There's a few reasons the kernel should not spam dmesg on bad
>>> userspace ioctl input:
>>> - at warning level it results in CI false positives
>>> - it allows userspace to drown dmesg output, potentially hiding real
>>>   issues.
>>>
>>> None of the other generic EINVAL checks report in the
>>> FBIOPUT_VSCREENINFO ioctl do this, so it's also inconsistent.
>>>
>>> I guess the intent of the patch which introduced this warning was that
>>> the drivers ->fb_check_var routine should fail in that case. Reality
>>> is that there's too many fbdev drivers and not enough people
>>> maintaining them by far, and so over the past few years we've simply
>>> handled all these validation gaps by tighning the checks in the core,
>>> because that's realistically really all that will ever happen.
>>>
>>> Reported-by: syzbot+20dcf81733d43ddff661 at syzkaller.appspotmail.com
>>> Link: https://syzkaller.appspot.com/bug?id=c5faf983bfa4a607de530cd3bb008888bf06cefc
>>> Fixes: 6c11df58fd1a ("fbmem: Check virtual screen sizes in fb_set_var()")
>>> Cc: Helge Deller <deller at gmx.de>
>>> Cc: Geert Uytterhoeven <geert at linux-m68k.org>
>>> Cc: stable at vger.kernel.org # v5.4+
>>> Cc: Daniel Vetter <daniel at ffwll.ch>
>>> Cc: Javier Martinez Canillas <javierm at redhat.com>
>>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>>> ---
>>>  drivers/video/fbdev/core/fbmem.c | 4 ----
>>>  1 file changed, 4 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>>> index 875541ff185b..9757f4bcdf57 100644
>>> --- a/drivers/video/fbdev/core/fbmem.c
>>> +++ b/drivers/video/fbdev/core/fbmem.c
>>> @@ -1021,10 +1021,6 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
>>>       /* verify that virtual resolution >= physical resolution */
>>>       if (var->xres_virtual < var->xres ||
>>>           var->yres_virtual < var->yres) {
>>> -             pr_warn("WARNING: fbcon: Driver '%s' missed to adjust virtual screen size (%ux%u vs. %ux%u)\n",
>>> -                     info->fix.id,
>>> -                     var->xres_virtual, var->yres_virtual,
>>> -                     var->xres, var->yres);
>>>               return -EINVAL;
>>>       }
>>>
>>
>> Make it pr_warn_once? 99.9...% of the benefit, without spam.
> 
> Except that it should be pr_warn_once_per_fb_info, [...]

Not really, that's what I mean by 99.9...% of the benefit. Even if a broken driver is masked on some systems, eventually the other driver masking it should get fixed, at which point the previously masked driver will be reported again.


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer