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

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

quoted

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.

quoted

 		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?

quoted

 		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?

quoted

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