On Tue, 4 Apr 2023 at 19:04, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Daniel, > > On Tue, Apr 4, 2023 at 5:55 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > > On Tue, 4 Apr 2023 at 16:51, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > On Tue, Apr 4, 2023 at 4:19 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > On Tue, Apr 04, 2023 at 03:59:12PM +0200, Daniel Vetter wrote: > > > > > On Tue, Apr 04, 2023 at 03:53:09PM +0200, Geert Uytterhoeven wrote: > > > > > > On Tue, Apr 4, 2023 at 2:36 PM Daniel Vetter <daniel.vetter@xxxxxxxx> 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 > > > > > > > > > > > > WARNING: fbcon: Driver 'vkmsdrmfb' missed to adjust virtual screen > > > > > > size (0x0 vs. 64x768) > > > > > > > > > > > > This is a bug in the vkmsdrmfb driver and/or DRM helpers. > > > > > > > > > > > > The message was added to make sure the individual drivers are fixed. > > > > > > Perhaps it should be changed to BUG() instead, so dmesg output > > > > > > cannot be drown? > > > > > > > > > > So you're solution is to essentially force us to replicate this check over > > > > > all the drivers which cannot change the virtual size? > > > > > > > > > > Are you volunteering to field that audit and type all the patches? > > > > > > > > Note that at least efifb, vesafb and offb seem to get this wrong. I didn't > > > > bother checking any of the non-fw drivers. Iow there is a _lot_ of work in > > > > your nack. > > > > > > Please don't spread FUD: efifb, vesafb and offb do not implement > > > fb_ops.fb_check_var(), so they are not affected. > > > > Hm I missed that early out. I'll do a patch to fix the drm fb helpers, > > as mentioned in the other thread I don't think we can actually just > > delete that because it would short-circuit out the fb_set_par call > > too. > > As I said to the other thread earlier today[1], I think we can keep > the .fb_set_par() implementation. > There's just no point in providing a .fb_check_var() callback if > you don't support changing the video mode. If you don't have check_var then set_par just isn't called. Or am blind once more, not the first time today. And see the big comment in the drm set_par implementation, we can't ditch that because uabi fun in how the fbdev vs kms interactions are handled. -Daniel > [1] https://lore.kernel.org/all/CAMuHMdUaHd1jgrsCSxCqF-HP2rAo2ODM_ZOjhk7Q4vjuqvt36w@xxxxxxxxxxxxxx > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
