[PATCH] fbcon: fix NULL pointer dereference for a console without vc_data

[PATCH] fbcon: fix NULL pointer dereference for a console without vc_data

[Ian Bridges]

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: syzbot+42525d636f430fd5d983@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=42525d636f430fd5d983
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
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/20260622080749.D7FC61F000E9@smtp.kernel.org/
[2] https://syzkaller.appspot.com/bug?extid=42525d636f430fd5d983

 drivers/video/fbdev/core/fbcon.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 9077d3b99357..97d4b836b26a 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1273,6 +1273,7 @@ static void fbcon_deinit(struct vc_data *vc)
 	int idx;
 
 	fbcon_free_font(p);
+	p->mode = NULL;
 	idx = con2fb_map[vc->vc_num];
 
 	if (idx == -1)
@@ -1443,14 +1444,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;
 
+	if (var_to_display(p, var, info))
+		return;
+
 	default_mode = vc->vc_display_fg;
 	svc = *default_mode;
 	t = &fb_display[svc->vc_num];
-- 
2.47.3

Re: [PATCH] fbcon: fix NULL pointer dereference for a console without vc_data

[Ian Bridges]

On Wed, Jun 24, 2026 at 09:30:26PM +0000, sashiko-bot@kernel.org wrote:
> 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 <icb@fastmail.org>
> 
> 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
> 
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index b0e3e765360d1..7187be0eea872 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -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():

This is a seperate bug that I'm also working on a patch for (although it
is proving a bit more difficult to find the correct place to make the
change). The NULL mode that fbcon_switch() reaches through
display_to_var() is set by var_to_display() at
drivers/video/fbdev/core/fbcon.c:988, which assigns disp->mode the result of
fb_match_mode(). On the no-match path that result is NULL, independent of
this patch. The p->mode = NULL added to fbcon_deinit() only applies to a
console that has been deinitialised and has no vc_data. fbcon_new_modelist()
skips such a console through its existing mode check, and the console cannot
be switched to without first being re-initialised, which re-runs
var_to_display() and overwrites the mode. So this patch does not add a path
that reaches display_to_var() with a NULL mode. The dereference is reached
when the modelist is replaced from userspace while fbcon is unbound.

> 
> 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?
> 
> >  	idx = con2fb_map[vc->vc_num];
> >  
> >  	if (idx == -1)
> 
> [ ... ]
> 
> > @@ -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;
> >  
> > +	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?
>

The block quoted sits below the hunk, and this patch only moves the vc_data
check ahead of var_to_display(). That block is reached only after
var_to_display() succeeds and vc is non-NULL, which holds in both the old and
the new order, so its reachability is unchanged with this patch.

Ian

> 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?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/ajxICA0v-_5NrqhS@dev?part=1