| Message ID | ZB4GS3zT3oh/afkf@ls3530 (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | fbdev: modedb: Fix kernel crash in fb_videomode_to_var() | expand |
On Fri, Mar 24, 2023 at 09:21:31PM +0100, Helge Deller wrote: > Fix a kernel crash in the fbdev modedb code which can happen if you boot > a system without any graphic card driver, in which case the dummycon > driver takes the console. If you then load a fbdev graphics driver and > start a the X11-fbdev the kernel will crash with a backtrace: > > IAOQ[0]: fb_videomode_to_var+0xc/0x88 > Backtrace: > [<10582ff8>] display_to_var+0x28/0xe8 > [<1058584c>] fbcon_switch+0x15c/0x55c > [<105a8a1c>] redraw_screen+0xdc/0x228 > [<1059d6f8>] complete_change_console+0x50/0x140 > [<1059eae0>] change_console+0x6c/0xdc > [<105ab4f4>] console_callback+0x1a0/0x1a8 > [<101cb5e8>] process_one_work+0x1c4/0x3cc > [<101cb978>] worker_thread+0x188/0x4b4 > [<101d5a94>] kthread+0xec/0xf4 > [<1018801c>] ret_from_kernel_thread+0x1c/0x24 > > The problem is, that the dummycon driver may not have a valid monitor > mode defined (which is ok for that driver), so the mode field in > fbcon_display can be NULL. > > Fix the crash by simply returning from fb_var_to_videomode() > if the video mode field is NULL. > > Signed-off-by: Helge Deller <deller@gmx.de> > Cc: stable <stable@kernel.org> Also here since the other thread is private: I don't think this makes sense, and it shouldn't happen that disp->mode is bogus when we have an fbdev bound for that vc. I think the below might work, I spotted this while auditing code around this (but it turned out to be a dead-end for the bug I was chasing): diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index eb565a10e5cd..1f2ab00ec6d4 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2684,8 +2684,8 @@ static void fbcon_modechanged(struct fb_info *info) p = &fb_display[vc->vc_num]; set_blitting_type(vc, info); + var_to_display(p, &info->var, info); if (con_is_visible(vc)) { - var_to_display(p, &info->var, info); cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres); rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); cols /= vc->vc_font.width; If this works I can bake it into a real patch. -Daniel > > --- > > diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c > index 6473e0dfe146..6a670b9dbcb4 100644 > --- a/drivers/video/fbdev/core/modedb.c > +++ b/drivers/video/fbdev/core/modedb.c > @@ -893,6 +893,8 @@ void fb_var_to_videomode(struct fb_videomode *mode, > void fb_videomode_to_var(struct fb_var_screeninfo *var, > const struct fb_videomode *mode) > { > + if (!mode) > + return; > var->xres = mode->xres; > var->yres = mode->yres; > var->xres_virtual = mode->xres;
On 4/12/23 20:36, Daniel Vetter wrote: > On Fri, Mar 24, 2023 at 09:21:31PM +0100, Helge Deller wrote: >> Fix a kernel crash in the fbdev modedb code which can happen if you boot >> a system without any graphic card driver, in which case the dummycon >> driver takes the console. If you then load a fbdev graphics driver and >> start a the X11-fbdev the kernel will crash with a backtrace: >> >> IAOQ[0]: fb_videomode_to_var+0xc/0x88 >> Backtrace: >> [<10582ff8>] display_to_var+0x28/0xe8 >> [<1058584c>] fbcon_switch+0x15c/0x55c >> [<105a8a1c>] redraw_screen+0xdc/0x228 >> [<1059d6f8>] complete_change_console+0x50/0x140 >> [<1059eae0>] change_console+0x6c/0xdc >> [<105ab4f4>] console_callback+0x1a0/0x1a8 >> [<101cb5e8>] process_one_work+0x1c4/0x3cc >> [<101cb978>] worker_thread+0x188/0x4b4 >> [<101d5a94>] kthread+0xec/0xf4 >> [<1018801c>] ret_from_kernel_thread+0x1c/0x24 >> >> The problem is, that the dummycon driver may not have a valid monitor >> mode defined (which is ok for that driver), so the mode field in >> fbcon_display can be NULL. >> >> Fix the crash by simply returning from fb_var_to_videomode() >> if the video mode field is NULL. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Cc: stable <stable@kernel.org> > > Also here since the other thread is private: I don't think this makes > sense, and it shouldn't happen that disp->mode is bogus when we have an > fbdev bound for that vc. > > I think the below might work, I spotted this while auditing code around > this (but it turned out to be a dead-end for the bug I was chasing): > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index eb565a10e5cd..1f2ab00ec6d4 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -2684,8 +2684,8 @@ static void fbcon_modechanged(struct fb_info *info) > p = &fb_display[vc->vc_num]; > set_blitting_type(vc, info); > > + var_to_display(p, &info->var, info); > if (con_is_visible(vc)) { > - var_to_display(p, &info->var, info); > cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres); > rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); > cols /= vc->vc_font.width; > > If this works I can bake it into a real patch. Daniel, thanks for your suggested patch! Actually I currently can't reproduce my original problem any longer. It happened with a specific fbdev driver which I'm currenlty working on, and in which I probably missed to set a valid video mode, which then led to the patch I originally sent. So, for now just ignore my patch. If it happens again I'll test your patch and report back. Thanks! Helge
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c index 6473e0dfe146..6a670b9dbcb4 100644 --- a/drivers/video/fbdev/core/modedb.c +++ b/drivers/video/fbdev/core/modedb.c @@ -893,6 +893,8 @@ void fb_var_to_videomode(struct fb_videomode *mode, void fb_videomode_to_var(struct fb_var_screeninfo *var, const struct fb_videomode *mode) { + if (!mode) + return; var->xres = mode->xres; var->yres = mode->yres; var->xres_virtual = mode->xres;
Fix a kernel crash in the fbdev modedb code which can happen if you boot a system without any graphic card driver, in which case the dummycon driver takes the console. If you then load a fbdev graphics driver and start a the X11-fbdev the kernel will crash with a backtrace: IAOQ[0]: fb_videomode_to_var+0xc/0x88 Backtrace: [<10582ff8>] display_to_var+0x28/0xe8 [<1058584c>] fbcon_switch+0x15c/0x55c [<105a8a1c>] redraw_screen+0xdc/0x228 [<1059d6f8>] complete_change_console+0x50/0x140 [<1059eae0>] change_console+0x6c/0xdc [<105ab4f4>] console_callback+0x1a0/0x1a8 [<101cb5e8>] process_one_work+0x1c4/0x3cc [<101cb978>] worker_thread+0x188/0x4b4 [<101d5a94>] kthread+0xec/0xf4 [<1018801c>] ret_from_kernel_thread+0x1c/0x24 The problem is, that the dummycon driver may not have a valid monitor mode defined (which is ok for that driver), so the mode field in fbcon_display can be NULL. Fix the crash by simply returning from fb_var_to_videomode() if the video mode field is NULL. Signed-off-by: Helge Deller <deller@gmx.de> Cc: stable <stable@kernel.org> ---