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

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

[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: 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 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/20260624213027.6C1E01F000E9@smtp.kernel.org/

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

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index e5221653ec2b..2f1c56e5a7a2 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -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;
-- 
2.47.3

Re: [PATCH] fbdev: Fix fb_new_modelist to prevent null-ptr-deref in fb_videomode_to_var

[sashiko-bot]

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 <icb@fastmail.org>

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.

> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index e5221653ec2b9..2f1c56e5a7a23 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c

[ ... ]

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

> +	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;

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