fbdev: Fix fb_new_modelist to prevent null-ptr-deref in fb_videomode_to_var

PATCH Applied Ian Bridges 3 participants 3 messages Jun 25 – Jun 26

Subsystems (2)

Framebuffer layer Framebuffer core

lore

Share & tools

lore

Ian Bridges

author

2026-06-25 04:13 UTC #0 lore raw

PATCH 1/1 fbdev: Fix fb_new_modelist to prevent null-ptr-deref in fb_videomode_to_var

info->var, a framebuffer's current mode, is expected to have a matching entry in info->modelist. var_to_display() relies on this and treats a failed fb_match_mode() as "This should not happen". fb_set_var() keeps it true by adding the mode to the list on every change, and do_register_framebuffer() does the same at registration.

store_modes() replaces the modelist from userspace. fb_new_modelist() validates the new modes but does not check that info->var still has a match. It relies on fbcon_new_modelist() to re-point consoles, but that only handles consoles mapped to the framebuffer. With fbcon unbound there are none, so info->var is left describing a mode that is no longer in the list.

A later console takeover runs var_to_display(), where fb_match_mode() returns NULL and leaves fb_display[i].mode NULL. fbcon_switch() passes it to display_to_var(), and fb_videomode_to_var() dereferences the NULL mode.

Keep the current mode in the list in fb_new_modelist(), the same way fb_set_var() does.

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 fb_videomode_to_var(), reached through the framebuffer console. The fix is in fb_new_modelist(). Sashiko flagged this dereference while reviewing the fix for a separate NULL pointer dereference in fbcon_new_modelist() [1].

The dereference happens when a framebuffer's current mode is dropped from its modelist while fbcon is unbound, and a console is then taken over onto it, as follows.

1. With fbcon unbound, a write to the modes attribute calls store_modes() (fbsysfs.c:91), which replaces the modelist and calls fb_new_modelist() (fbsysfs.c:108). fb_new_modelist() (fbmem.c:746) validates the new modes but does not keep info->var in the list, and the fbcon_new_modelist() it calls (fbmem.c:770) only re-points consoles mapped to the framebuffer. With fbcon unbound there are none, so info->var is left describing a mode no longer in the list.
2. Mapping a console with FBIOPUT_CON2FBMAP takes the framebuffer over, set_con2fb_map() -> do_fbcon_takeover() (fbcon.c:930).
3. The takeover initialises the console, fbcon_init() -> var_to_display() (fbcon.c:1113). fb_match_mode() finds no entry for info->var, so var_to_display() leaves fb_display[i].mode NULL (fbcon.c:988).
4. The takeover switches to the console, fbcon_switch() -> display_to_var() (fbcon.c:2181), and fb_videomode_to_var() reads the NULL mode (modedb.c:905). This is a NULL pointer dereference.

The faulting line is not touched by this patch:

    var->xres = mode->xres;

mode is the fb_display[i].mode passed by display_to_var(). Instead of guarding this read, the patch keeps info->var in the modelist in fb_new_modelist(), so the mode is never NULL here.

The same dereference, fb_videomode_to_var() on a NULL mode, was fixed twice before, by CVE-2025-38214 in fb_set_var() and CVE-2025-38215 in do_register_framebuffer(). Both keep info->var in the modelist, and both are already in this base. This bug reaches the same line through a different path, a modelist replacement, and this fix keeps info->var in the list the same way.

It reproduces on a KASAN kernel with two framebuffers:

1. Unbind fbcon, so the store in step 3 does not re-sync the mode.
2. Set a video mode on fb0 with FBIOPUT_VSCREENINFO.
3. Write a different modelist to /sys/class/graphics/fb0/modes, so the mode from step 2 is no longer in the modelist.
4. Map a console to fb0 with FBIOPUT_CON2FBMAP. With fbcon unbound this takes fb0 over, sets the console mode to NULL (the mode is not in the modelist), and switches to the console.

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 defect is present in the initial 2.6.12-rc2 import, so there is no Fixes tag.

[1] https://lore.kernel.org/all/[email protected]/

 drivers/video/fbdev/core/fbmem.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/video/fbdev/core/fbmem.c +12 -0

index e5221653ec2b..2f1c56e5a7a2 100644
@@ -767,6 +767,18 @@ int fb_new_modelist(struct fb_info *info)
 	if (list_empty(&info->modelist))
 		return 1;
 
+	/*
+	 * The new modelist may not contain the current mode (info->var), and
+	 * fbcon_new_modelist() below only re-points consoles mapped to this
+	 * framebuffer. Add the current mode here so info->var keeps a match
+	 * even when fbcon is unbound.
+	 */
+	if (!fb_match_mode(&info->var, &info->modelist)) {
+		fb_var_to_videomode(&mode, &info->var);
+		if (fb_add_videomode(&mode, &info->modelist))
+			return 1;
+	}
+
 	fbcon_new_modelist(info);
 
 	return 0;

Signature · 1 line

2.47.3

[email protected]

AI review

2026-06-25 04:26 UTC #1 lore raw

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

Pre-existing issues:
- [High] Buffer overflow in show_modes() due to unbounded accumulation of offset. --

commit c6eace86a62190121a8b0640238eca3a23f8495d
Author: Ian Bridges <[email protected]>

fbdev: Fix fb_new_modelist to prevent null-ptr-deref in fb_videomode_to_var

This commit fixes a null pointer dereference in fb_videomode_to_var by ensuring that the current mode is kept in the modelist during fb_new_modelist() if it is not already present.

Quoted 4 lines

drivers/video/fbdev/core/fbmem.c +0 -0
index e5221653ec2b9..2f1c56e5a7a23 100644

[ ... ]

drivers/video/fbdev/core/fbmem.c int fb_new_modelist(struct fb_info *info) · line 775

Quoted 10 lines

drivers/video/fbdev/core/fbmem.c +6 -0

@@ -767,6 +767,18 @@ int fb_new_modelist(struct fb_info *info)
 	if (list_empty(&info->modelist))
 		return 1;
 
+	/*
+	 * The new modelist may not contain the current mode (info->var), and
+	 * fbcon_new_modelist() below only re-points consoles mapped to this
+	 * framebuffer. Add the current mode here so info->var keeps a match
+	 * even when fbcon is unbound.
+	 */

[Severity: High]
This is a pre-existing issue, but while reviewing how modelists are populated here, I noticed an older problem when the modelist is read from sysfs.

Does this code overflow the sysfs buffer on the kernel heap?

If a driver populates the framebuffer's modelist with many entries, or root writes them via the store_modes sysfs interface, the offset i accumulates the return value of mode_string() continuously in show_modes():

drivers/video/fbdev/core/fbsysfs.c:show_modes() {
    ...
    i += mode_string(buf, i, mode);
    ...
}

The mode_string() function passes this offset directly to snprintf:

drivers/video/fbdev/core/fbsysfs.c:mode_string() {
    ...
    return snprintf(&buf[offset], PAGE_SIZE - offset, "%c:%dx%d%c-%d\n",
                    m, mode->xres, mode->yres, v, mode->refresh);
}

Because snprintf returns the number of characters that would have been written regardless of truncation, the offset will eventually exceed PAGE_SIZE if there are enough modes.

When offset is greater than PAGE_SIZE, the expression PAGE_SIZE - offset integer-underflows to a massive positive size_t value, causing snprintf to write past the 1-page sysfs buffer allocation.

Quoted 9 lines

drivers/video/fbdev/core/fbmem.c +6 -0

+	if (!fb_match_mode(&info->var, &info->modelist)) {
+		fb_var_to_videomode(&mode, &info->var);
+		if (fb_add_videomode(&mode, &info->modelist))
+			return 1;
+	}
+
 	fbcon_new_modelist(info);
 
 	return 0;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/ajyq2Fr-2fMfftGC@dev?part=1

Helge Deller

maintainer Applied

2026-06-26 13:15 UTC #2 lore raw

In reply to Ian Bridges

Quoted 80 lines · Ian Bridges

info->var, a framebuffer's current mode, is expected to have a matching entry in info->modelist. var_to_display() relies on this and treats a failed fb_match_mode() as "This should not happen". fb_set_var() keeps it true by adding the mode to the list on every change, and do_register_framebuffer() does the same at registration.
store_modes() replaces the modelist from userspace. fb_new_modelist() validates the new modes but does not check that info->var still has a match. It relies on fbcon_new_modelist() to re-point consoles, but that only handles consoles mapped to the framebuffer. With fbcon unbound there are none, so info->var is left describing a mode that is no longer in the list.
A later console takeover runs var_to_display(), where fb_match_mode() returns NULL and leaves fb_display[i].mode NULL. fbcon_switch() passes it to display_to_var(), and fb_videomode_to_var() dereferences the NULL mode.
Keep the current mode in the list in fb_new_modelist(), the same way fb_set_var() does.
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 fb_videomode_to_var(), reached through the framebuffer console. The fix is in fb_new_modelist(). Sashiko flagged this dereference while reviewing the fix for a separate NULL pointer dereference in fbcon_new_modelist() [1].
The dereference happens when a framebuffer's current mode is dropped from its modelist while fbcon is unbound, and a console is then taken over onto it, as follows.
1. With fbcon unbound, a write to the modes attribute calls store_modes() (fbsysfs.c:91), which replaces the modelist and calls fb_new_modelist() (fbsysfs.c:108). fb_new_modelist() (fbmem.c:746) validates the new modes but does not keep info->var in the list, and the fbcon_new_modelist() it calls (fbmem.c:770) only re-points consoles mapped to the framebuffer. With fbcon unbound there are none, so info->var is left describing a mode no longer in the list.
2. Mapping a console with FBIOPUT_CON2FBMAP takes the framebuffer over,
    set_con2fb_map() -> do_fbcon_takeover() (fbcon.c:930).
3. The takeover initialises the console, fbcon_init() -> var_to_display()
    (fbcon.c:1113). fb_match_mode() finds no entry for info->var, so
    var_to_display() leaves fb_display[i].mode NULL (fbcon.c:988).
4. The takeover switches to the console, fbcon_switch() -> display_to_var() (fbcon.c:2181), and fb_videomode_to_var() reads the NULL mode (modedb.c:905). This is a NULL pointer dereference.
The faulting line is not touched by this patch:
     var->xres = mode->xres;
mode is the fb_display[i].mode passed by display_to_var(). Instead of guarding this read, the patch keeps info->var in the modelist in fb_new_modelist(), so the mode is never NULL here.
The same dereference, fb_videomode_to_var() on a NULL mode, was fixed twice before, by CVE-2025-38214 in fb_set_var() and CVE-2025-38215 in do_register_framebuffer(). Both keep info->var in the modelist, and both are already in this base. This bug reaches the same line through a different path, a modelist replacement, and this fix keeps info->var in the list the same way.
It reproduces on a KASAN kernel with two framebuffers:
1. Unbind fbcon, so the store in step 3 does not re-sync the mode.
2. Set a video mode on fb0 with FBIOPUT_VSCREENINFO.
3. Write a different modelist to /sys/class/graphics/fb0/modes, so the mode from step 2 is no longer in the modelist.
4. Map a console to fb0 with FBIOPUT_CON2FBMAP. With fbcon unbound this takes fb0 over, sets the console mode to NULL (the mode is not in the modelist), and switches to the console.
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 defect is present in the initial 2.6.12-rc2 import, so there is no Fixes tag.

Quoted 4 lines

[1] https://lore.kernel.org/all/[email protected]/
  drivers/video/fbdev/core/fbmem.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

applied.
Thanks!
Helge