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 [off-list ref]

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

quoted

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?

quoted

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

[ ... ]

quoted

@@ -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