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@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx >>> Link: https://syzkaller.appspot.com/bug?id=c5faf983bfa4a607de530cd3bb008888bf06cefc >>> Fixes: 6c11df58fd1a ("fbmem: Check virtual screen sizes in fb_set_var()") >>> Cc: Helge Deller <deller@xxxxxx> >>> Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> >>> Cc: stable@xxxxxxxxxxxxxxx # v5.4+ >>> Cc: Daniel Vetter <daniel@xxxxxxxx> >>> Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx> >>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> >>> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> >>> --- >>> 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
