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

On 6/25/26 06:13, Ian Bridges wrote:
> 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(+)

applied.
Thanks!
Helge