Hi Michel,
On Wed, Apr 5, 2023 at 10:50 AM Michel Dänzer michel.daenzer@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@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=c5faf983bfa4a607de530cd3bb008888bf06cef... Fixes: 6c11df58fd1a ("fbmem: Check virtual screen sizes in fb_set_var()") Cc: Helge Deller deller@gmx.de Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: stable@vger.kernel.org # v5.4+ Cc: Daniel Vetter daniel@ffwll.ch Cc: Javier Martinez Canillas javierm@redhat.com Cc: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Daniel Vetter daniel.vetter@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, which requires a flag in fb_info.
If fb_info.fbops wasn't const, we could also nuke info->fbops->check_var() ;-)
Gr{oetje,eeting}s,
Geert