Re: [PATCH] fbcon: fix NULL pointer dereference for a console without vc_data
Helge Deller <[email protected]>
| Newsgroups | org.kernel.vger.linux-fbdev,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 6/24/26 23:11, Ian Bridges wrote: > fbcon_new_modelist() runs when a framebuffer's modelist changes. For each > console mapped to it with fb_display[i].mode set, it reads vc_cons[i].d and > passes the vc_num to fbcon_set_disp(). This assumes a console with a mode > set has a vc_data, but it can be NULL. fbcon_set_disp() sets > fb_display[i].mode before it checks vc_data, and fbcon_deinit() leaves the > mode set after the vc_data is freed. fbcon_new_modelist() then dereferences > the NULL vc_data. > > Keep fb_display[i].mode set only while the console has a vc_data. Check > vc_data before setting the mode in fbcon_set_disp(), and clear the mode in > fbcon_deinit(). The existing mode check in fbcon_new_modelist() then skips > such consoles. > > Reported-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=42525d636f430fd5d983 > Cc: [email protected] > Assisted-by: Claude:claude-opus-4-8 > Signed-off-by: Ian Bridges <[email protected]> > --- > This patch fixes a NULL pointer dereference in the framebuffer console code. > fbcon_new_modelist() dereferences a NULL vc_data. It was found while writing > a reproducer for a separate use-after-free in store_modes(). Sashiko > independently flagged the same dereference in its review of the fix for that > use-after-free [1]. > > The dereference happens when a console has fb_display[i].mode set but no > vc_data, and the modelist is then replaced, as follows. > > 1. A console ends up with fb_display[i].mode set while vc_cons[i].d is NULL. > Either fbcon_set_disp() sets the mode (fbcon.c:1446) before it checks > vc_data, so mapping an unused console with FBIOPUT_CON2FBMAP leaves the > mode set, or fbcon_deinit() (fbcon.c:1268) frees the vc_data but keeps the > mode. > 2. A write to the modes attribute calls store_modes() (fbsysfs.c:91), which > replaces the modelist and calls fb_new_modelist() (fbsysfs.c:108). > 3. fb_new_modelist() calls fbcon_new_modelist() (fbmem.c:770). > 4. fbcon_new_modelist() walks the consoles mapped to the framebuffer, takes > vc = vc_cons[i].d for the one with the mode set, and reads vc->vc_num with > vc NULL (fbcon.c:3046). This is a NULL pointer dereference. > > This patch does not change that line: > > fbcon_set_disp(info, &var, vc->vc_num); > > vc is vc_cons[i].d, which is NULL. > > syzbot reported the same crash, with the call stack store_modes() -> > fb_new_modelist() -> fbcon_new_modelist() [2]. That report had no reproducer > and was closed as obsolete. > > It reproduces on a KASAN kernel with two framebuffers, two ways: > > 1. Map an unused console to one framebuffer and back to the other with > FBIOPUT_CON2FBMAP, then write a modelist to that framebuffer's modes > attribute. > 2. Open and close /dev/ttyN, then write a modelist to its framebuffer's > modes attribute. > > The reproducer was written with the help of a coding agent (Claude Code). The > patch is against commit 3726ce7f6cef on the for-next branch of > git://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git. The > file offsets above are from that commit. The dereference is present in the > initial 2.6.12-rc2 import, so there is no Fixes tag. > > [1] https://lore.kernel.org/all/[email protected]/ > [2] https://syzkaller.appspot.com/bug?extid=42525d636f430fd5d983 > > drivers/video/fbdev/core/fbcon.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) applied. Thanks! Helge