[PATCH] fbdev: fix use-after-free of fb_display[i].mode in store_modes()

[PATCH] fbdev: fix use-after-free of fb_display[i].mode in store_modes()

[Ian Bridges]

store_modes() replaces a framebuffer's modelist and frees the old entries
with fb_destroy_modelist(). fb_display[i].mode is a back-pointer into a
modelist entry, and it is not cleared when a console is detached from the
framebuffer (unbinding fbcon clears con2fb_map[] but leaves
fb_display[i].mode set).

store_modes() relies on fb_new_modelist() -> fbcon_new_modelist() to move
the back-pointers to the new list, but that only re-points consoles still
mapped to the framebuffer. A console that is no longer mapped to it is
skipped, so once fb_destroy_modelist() frees the old list that console's
fb_display[i].mode dangles. A later FBIOPUT_VSCREENINFO with
FB_ACTIVATE_INV_MODE makes fbcon_mode_deleted() read it through
fb_mode_is_equal(), a use-after-free.

Commit a1f305893074 ("fbcon: Set fb_display[i]->mode to NULL when the
mode is released") fixed the same use-after-free for the framebuffer
unregister path. Do the same in store_modes() by clearing the stale
pointers with fbcon_delete_modelist() before freeing the old list.

Reported-by: syzbot+81c7c6b52649fd07299d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=81c7c6b52649fd07299d
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
This patch contains a proposed fix for a crash reported by syzbot in
fb_mode_is_equal().

The file names and offsets in this description are from commit
8cd9520d35a6c38db6567e97dd93b1f11f185dc6 on the for-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git

I also have a deterministic reproducer that triggers the crash under virtme-ng
with a KASAN kernel and two framebuffers (vfb as fb0, bochs-drm as fb1). It
performs six ioctl/sysfs operations, listed at the end of "The Bug". The free
is driven by a write to /sys/class/graphics/fb0/modes, which needs write access
to that file. The reproducer was written with the help of a coding agent
(Claude Code).

This crash has the same KASAN signature as the one fixed by commit
a1f305893074 ("fbcon: Set fb_display[i]->mode to NULL when the mode is
released"), but reaches it through a different free path. It is still reported
on kernels that contain that commit.

The Bug

fb_display[i].mode is a back-pointer into a struct fb_modelist entry on
info->modelist. var_to_display() (fbcon.c:989) sets it when a console is
attached to a framebuffer. fbcon_mode_deleted() reads it through
fb_mode_is_equal() (fbcon.c:2750, modedb.c:931).

store_modes() is the .store handler for /sys/class/graphics/fbN/modes
(fbsysfs.c:94). It splices the current modelist into a local old_list, builds
the new list, and on success frees old_list with fb_destroy_modelist()
(fbsysfs.c:115). To keep the back-pointers valid it relies on
fb_new_modelist() -> fbcon_new_modelist() (fbmem.c:737), but that only re-points
consoles still mapped to this framebuffer (fbcon_info_from_console(i) != info,
fbcon.c:3038).

fb_display[i].mode is not cleared when a console is detached. Unbinding fbcon
runs fbcon_release_all() (fbcon.c:1248), which sets con2fb_map[i] = -1 but
leaves fb_display[i].mode pointing into the former framebuffer's modelist. Such
a console is skipped by fbcon_new_modelist(), so once fb_destroy_modelist()
frees old_list its fb_display[i].mode dangles.

The minimal reproduction is six operations on two framebuffers (fb0, fb1). All
six are required (verified against source and by removing each in turn). fbcon
starts bound with the consoles mapped to fb0. The operations all act on one
unused console i (no allocated vc_data). An allocated console would be
re-initialised, and its fb_display[i].mode reset, by the takeover in step 5.
Pre-allocating all consoles does not reproduce.

  1. FBIOPUT_CON2FBMAP, the console -> fb1
     set_con2fb_map() reaches var_to_display(), which points fb_display[i].mode
     at an fb1 modelist entry (var_to_display() uses the console index, so it
     runs even though the console has no vc_data)
  2. FBIOPUT_CON2FBMAP, the console -> fb0
     fb_display[i].mode now points at an fb0 modelist entry M
  3. echo 0 > /sys/class/vtconsole/vtcon1/bind
     fbcon_release_all() sets con2fb_map[i] = -1 and leaves fb_display[i].mode
     pointing at M
  4. write a videomode array to /sys/class/graphics/fb0/modes
     The buffer holds raw struct fb_videomode entries. At least one must be
     valid so fb_new_modelist() keeps it and store_modes() takes the success
     path that frees old_list (an empty or invalid set takes the failure path
     that restores it, and nothing is freed). fbcon_new_modelist() skips the
     console (con2fb_map[i] == -1), then fb_destroy_modelist() frees M, so
     fb_display[i].mode now dangles
  5. FBIOPUT_CON2FBMAP, the console -> fb1
     fbcon is unbound, so set_con2fb_map() takes the do_fbcon_takeover() branch
     (fbcon.c:931). do_fbcon_takeover() sets con2fb_map[] = info_idx for the
     whole range (fbcon.c:619), so the console is now mapped to fb1. The mode is
     reset only via fbcon_init() during the takeover, which runs for allocated
     consoles only, so this unused console keeps its dangling fb_display[i].mode
  6. FBIOPUT_VSCREENINFO on fb1 with FB_ACTIVATE_INV_MODE and a mode different
     from fb1's current var (otherwise fb_set_var() returns early without
     calling fbcon_mode_deleted(), to avoid deleting the current var's
     videomode)
     fb_set_var() (fbmem.c:240,248) -> fbcon_mode_deleted() walks fb1's consoles
     and reads the dangling fb_display[i].mode via fb_mode_is_equal(), a
     use-after-free

The free is in store_modes() on fb0, and the read in fbcon_mode_deleted() on
fb1.

The Proposed Fix

A framebuffer's modelist is freed in two places, do_unregister_framebuffer()
(fbmem.c:548) and store_modes() (fbsysfs.c:115). Commit a1f305893074 added the
helper fbcon_delete_modelist() to clear the stale fb_display[i].mode pointers,
but called it from the first site only (fbmem.c:547). The store_modes() site
was raised as a potential issue during review of that commit [1] but
assumed safe.

Fix store_modes() the same way by clearing the stale pointers with
fbcon_delete_modelist() before freeing old_list.

The fix was verified with the reproducer. The unpatched kernel reports the
slab-use-after-free read, freed by fb_destroy_modelist() from store_modes().
The patched kernel produces no use-after-free.

The bug predates git history. Both the fb_display[i].mode back-pointer and the
store_modes() swap-and-free are present in the initial 2.6.12-rc2 import
(1da177e4c3f4), so there is no Fixes tag, matching a1f305893074. The fix calls
fbcon_delete_modelist(), which a1f305893074 added, so a stable backport needs
that commit first.

[1] https://lkml.org/lkml/2025/9/23/786

 drivers/video/fbdev/core/fbsysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index baa2bae0fb5b..c761dcf21daf 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -111,8 +111,10 @@ static ssize_t store_modes(struct device *device,
 	if (fb_new_modelist(fb_info)) {
 		fb_destroy_modelist(&fb_info->modelist);
 		list_splice(&old_list, &fb_info->modelist);
-	} else
+	} else {
+		fbcon_delete_modelist(&old_list);
 		fb_destroy_modelist(&old_list);
+	}
 
 	unlock_fb_info(fb_info);
 	console_unlock();
-- 
2.47.3

Re: [PATCH] fbdev: fix use-after-free of fb_display[i].mode in store_modes()

[Ian Bridges]

On Mon, Jun 22, 2026 at 08:07:49AM +0000, sashiko-bot@kernel.org wrote:

I'm new to submitting kernel patches, so please correct me if I have
handled this wrong. All four findings are pre-existing and none are
introduced by this patch, which I scoped to the reported
fb_display[i].mode use-after-free. Responses inline.

> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Use-After-Free in show_mode() due to dangling fb_info->mode pointer.
> - [High] Data Race and potential Use-After-Free / list corruption when iterating fb_info->modelist.
> - [High] NULL pointer dereference in fbcon_new_modelist() for unallocated consoles.
> - [Medium] Data Race and locking violation modifying fb_display in do_unregister_framebuffer().
> --
> 
> commit 5e256dfc1372231ec16feb6ee8a3218859b0dd4f
> Author: Ian Bridges <icb@fastmail.org>
> 
> fbdev: fix use-after-free of fb_display[i].mode in store_modes()
> 
> store_modes() replaces a framebuffer's modelist and frees the old entries
> with fb_destroy_modelist(), but fb_display[i].mode was left dangling.
> This commit fixes the use-after-free by clearing the stale pointers with
> fbcon_delete_modelist() before freeing the old list.
> 
> Closes: https://syzkaller.appspot.com/bug?extid=81c7c6b52649fd07299d
> 
> > diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
> > index baa2bae0fb5b3..c761dcf21dafb 100644
> > --- a/drivers/video/fbdev/core/fbsysfs.c
> > +++ b/drivers/video/fbdev/core/fbsysfs.c
> > @@ -111,8 +111,10 @@ static ssize_t store_modes(struct device *device,
> >  	if (fb_new_modelist(fb_info)) {
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can this fb_new_modelist()
> call lead to a NULL pointer dereference for unallocated consoles?
> 
> If an unallocated console (where vc_cons[i].d is NULL) is mapped to a
> framebuffer, fbcon_set_disp() populates fb_display[i].mode and returns early.
> 
> When store_modes() calls fb_new_modelist() -> fbcon_new_modelist(),
> it checks if con2fb_map and fb_display[i].mode are set. Could it then fetch
> vc = vc_cons[i].d and unconditionally dereference vc->vc_num?
> 
> drivers/video/fbdev/core/fbcon.c:fbcon_new_modelist() {
>     ...
>     vc = vc_cons[i].d;
>     ...
>     fbcon_set_disp(info, &var, vc->vc_num);
>     ...
> }

Confirmed. fbcon_new_modelist() dereferences vc_cons[i].d without a NULL
check, and a console can have con2fb_map and fb_display[i].mode set while
vc_cons[i].d is NULL. I hit this while writing the reproducer for this bug.
It is a separate issue, so I'd argue it needs its own patch adding the NULL
check.

> 
> >  		fb_destroy_modelist(&fb_info->modelist);
> >  		list_splice(&old_list, &fb_info->modelist);
> > -	} else
> > +	} else {
> > +		fbcon_delete_modelist(&old_list);
> 
> [Severity: Medium]
> This is a pre-existing issue, but since we are properly calling
> fbcon_delete_modelist() under console_lock() here, should other callers do
> the same?
> 
> Looking at do_unregister_framebuffer(), it executes without holding
> console_lock():
> 
> drivers/video/fbdev/core/fbmem.c:do_unregister_framebuffer() {
>     ...
>     fbcon_delete_modelist(&fb_info->modelist);
>     ...
> }

Agreed. do_unregister_framebuffer() runs under registration_lock, not
console_lock, so the fbcon_delete_modelist() call added by a1f305893074
writes fb_display[] without console_lock. The call this patch adds to
store_modes() runs under console_lock and is safe. The unregister call site
looks worth a separate fix.

> 
> Since fbcon_delete_mode() iterates the global fb_display array and locklessly
> assigns NULL to fb_display[i].mode, could this cause a data race with
> concurrent readers like FBIOPUT_VSCREENINFO?
> 
> >  		fb_destroy_modelist(&old_list);
> 
> [Severity: High]
> This is a pre-existing issue, but does destroying old_list leave
> fb_info->mode dangling?
> 
> If fb_info->mode was previously set to point into a struct fb_modelist
> element from this list, it doesn't appear to be cleared before the list
> is freed.
> 
> Could a subsequent read of the sysfs mode attribute trigger a use-after-free
> here:
> 
> drivers/video/fbdev/core/fbsysfs.c:show_mode() {
>     ...
>     return mode_string(buf, 0, fb_info->mode);
> }

Confirmed, and it still reproduces with this patch applied. store_modes()
frees the old modelist that fb_info->mode points into, and this patch only
clears fb_display[i].mode. Writing a mode to /sys/class/graphics/fb0/mode,
then a new list to /sys/class/graphics/fb0/modes, then reading
/sys/class/graphics/fb0/mode faults in mode_string() called from
show_mode(). Since it is the same free in store_modes(), I will fold the
fix into a v2 of this patch and clear fb_info->mode alongside
fb_display[i].mode.

> 
> [Severity: High]
> This is also a pre-existing issue, but does modifying fb_info->modelist
> under lock_fb_info() race with sysfs readers?
> 
> Looking at show_modes() and store_mode(), they appear to iterate
> fb_info->modelist locklessly:
> 
> drivers/video/fbdev/core/fbsysfs.c:show_modes() {
>     ...
>     list_for_each_entry(modelist, &fb_info->modelist, list) {
>     ...
> }

Agreed. show_modes() and store_mode() walk fb_info->modelist without
lock_fb_info(), so they can race the splice and free in store_modes(). This
looks like a separate locking fix.

> 
> Since store_modes() splices, modifies, and destroys this list, could a
> concurrent sysfs read/write follow corrupted pointers into freed memory?
> 
> > +	}
> >  
> >  	unlock_fb_info(fb_info);
> >  	console_unlock();
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/ajjoDhAi2y4ArSlz@dev?part=1

To summarise, I will send a v2 of this patch that clears both stale pointers
in store_modes() (fb_display[i].mode and fb_info->mode), and keep the
fbcon_new_modelist() NULL check and the two locking issues as separate
patches.

Thanks,
Ian