fbcon: fix NULL pointer dereference for a console without vc_data

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing NULL pointer dereference via font_data_get(NULL) in fbcon_set_disp() if the foreground console's font is uninitialized.
- [High] Pre-existing NULL pointer dereference in fb_videomode_to_var() when a console is switched to or resized while its mode is NULL. --

commit 6f0719a9bd0a2231fe53bfe57ca1abf9ddec2785
Author: Ian Bridges <[email protected]>

fbcon: fix NULL pointer dereference for a console without vc_data

This commit fixes a NULL pointer dereference in fbcon_new_modelist(). It does this by ensuring a console's mode is only set while it has a valid vc_data structure.

Closes https://syzkaller.appspot.com/bug?extid=42525d636f430fd5d983

drivers/video/fbdev/core/fbcon.c static void fbcon_deinit(struct vc_data *vc) · line 1277

Quoted 9 lines

drivers/video/fbdev/core/fbcon.c +1 -0

index b0e3e765360d1..7187be0eea872 100644
@@ -1274,6 +1274,7 @@ static void fbcon_deinit(struct vc_data *vc)
 	int idx;
 
 	fbcon_free_font(p);
+	p->mode = NULL;

[Severity: High]
This is a pre-existing issue, but does setting p->mode to NULL here expose us to a crash during a VT switch?

If a subsequent fbcon_init() fails early (e.g. var_to_display() fails to find a matching mode), p->mode will remain NULL. Later, if a user switches to this VT, the VT layer calls fbcon_switch(), which unconditionally passes p to display_to_var():

drivers/video/fbdev/core/fbcon.c:fbcon_switch() {
    ...
    memset(&var, 0, sizeof(struct fb_var_screeninfo));
    display_to_var(&var, p);
    ...
}

display_to_var() then calls fb_videomode_to_var():

drivers/video/fbdev/core/fbcon.c:display_to_var() {
    ...
    fb_videomode_to_var(var, disp->mode);
    ...
}

And fb_videomode_to_var() immediately dereferences the pointer:

drivers/video/fbdev/core/modedb.c:fb_videomode_to_var() {
    ...
    var->xres = mode->xres;
    ...
}

Can this lead to a NULL pointer dereference if the modelist is altered from userspace?

Quoted 3 lines

drivers/video/fbdev/core/fbcon.c +0 -0

 	idx = con2fb_map[vc->vc_num];
 
 	if (idx == -1)

[ ... ]

Quoted 12 lines

drivers/video/fbdev/core/fbcon.c +0 -3

@@ -1445,14 +1446,14 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
 
 	p = &fb_display[unit];
 
-	if (var_to_display(p, var, info))
-		return;
-
 	vc = vc_cons[unit].d;
 
 	if (!vc)
 		return;
 

drivers/video/fbdev/core/fbcon.c static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var, · line 1457

Quoted 6 lines

drivers/video/fbdev/core/fbcon.c +3 -0

+	if (var_to_display(p, var, info))
+		return;
+
 	default_mode = vc->vc_display_fg;
 	svc = *default_mode;
 	t = &fb_display[svc->vc_num];

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer dereference in font_data_get() if the foreground console's font is uninitialized?

Just below this hunk in fbcon_set_disp(), the code does:

    if (!p->fontdata) {
        ...
        p->fontdata = t->fontdata;
        font_data_get(p->fontdata);
    }

If fbcon_init() previously failed early for the foreground console, its t->fontdata could be NULL. This assigns NULL to p->fontdata and then passes it to font_data_get():

lib/fonts/fonts.c:font_data_get() {
if (font_data_is_internal(fd))
return; /* never ref static data */

if (WARN_ON(!REFCOUNT(fd)))
return; /* should never be 0 */
++REFCOUNT(fd);
}

The REFCOUNT() macro and font_data_is_internal() will unconditionally dereference fd, causing a crash if it is NULL. Does this need an extra NULL check?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/ajxICA0v-_5NrqhS@dev?part=1

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(-)