drivers/video/fbdev/core/fbcon.c | 11 +++++++++++ drivers/video/fbdev/core/modedb.c | 7 +++++++ include/linux/fbcon.h | 2 ++ 3 files changed, 20 insertions(+)
Recently, we discovered the following issue through syzkaller:
BUG: KASAN: slab-use-after-free in fb_mode_is_equal+0x285/0x2f0
Read of size 4 at addr ff11000001b3c69c by task syz.xxx
...
Call Trace:
<TASK>
dump_stack_lvl+0xab/0xe0
print_address_description.constprop.0+0x2c/0x390
print_report+0xb9/0x280
kasan_report+0xb8/0xf0
fb_mode_is_equal+0x285/0x2f0
fbcon_mode_deleted+0x129/0x180
fb_set_var+0xe7f/0x11d0
do_fb_ioctl+0x6a0/0x750
fb_ioctl+0xe0/0x140
__x64_sys_ioctl+0x193/0x210
do_syscall_64+0x5f/0x9c0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
The issue occurs in the function fb_mode_is_equal(p->mode, mode), I also
noticed that when freeing the memory related to fb_info->modelist, there's
no attempt to set the corresponding fb_display[i]->mode to NULL after
freeing. Based on analysis, the root cause of this bug appears to be that
a certain p->mode has become a wild pointer.
I've identified two code paths for freeing modelist->mode:
1. fb_delete_videomode - removes videomode entry from modelist.
2. fb_destroy_modelist - destroys the entire modelist.
Analysis shows that fb_delete_videomode path should have been fixed in
a previous patch[1]. Therefore, the current bug is likely triggered
through the fb_destroy_modelist path. I've found a reproducible test case:
1. With /dev/fb0 already registered in the system, load a kernel module
to register a new device /dev/fb1;
2. Set fb1's mode to the global fb_display[] array (via FBIOPUT_CON2FBMAP);
3. Switch console from fb to VGA (to allow normal rmmod of the ko);
4. Unload the kernel module - at this point fb1's modelist is freed, leaving
a wild pointer in fb_display[];
5. Trigger the bug via system calls through fb0 attempting to delete a mode
from fb0.
To prevent similar issues from recurring, consider traversing fb_display[]
whenever releasing a mode from fb_info. If the corresponding mode exists
in fb_display[], set its pointer to NULL.
[1] https://lore.kernel.org/all/20210712085544.2828-1-thunder.leizhen@huawei.com/
Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com>
---
This is my first time working on fb issues. If there are any misunderstandings
in my analysis, I would appreciate corrections from the community.
drivers/video/fbdev/core/fbcon.c | 11 +++++++++++
drivers/video/fbdev/core/modedb.c | 7 +++++++
include/linux/fbcon.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index b062b05f4128..bfbf79d6cd05 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2803,6 +2803,17 @@ int fbcon_mode_deleted(struct fb_info *info,
return found;
}
+void fb_display_clean_videomode(struct fb_videomode *m)
+{
+ struct fbcon_display *p;
+
+ for (int i = first_fb_vc; i <= last_fb_vc; i++) {
+ p = &fb_display[i];
+ if (p->mode == m)
+ p->mode = NULL;
+ }
+}
+
#ifdef CONFIG_VT_HW_CONSOLE_BINDING
static void fbcon_unbind(void)
{
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index 53a610948c4a..5a0ee96ebefa 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -16,6 +16,7 @@
#include <linux/slab.h>
#include <linux/fb.h>
#include <linux/kernel.h>
+#include <linux/fbcon.h>
#undef DEBUG
@@ -1100,6 +1101,7 @@ void fb_delete_videomode(const struct fb_videomode *mode,
modelist = list_entry(pos, struct fb_modelist, list);
m = &modelist->mode;
if (fb_mode_is_equal(m, mode)) {
+ fb_display_clean_videomode(m);
list_del(pos);
kfree(pos);
}
@@ -1113,8 +1115,13 @@ void fb_delete_videomode(const struct fb_videomode *mode,
void fb_destroy_modelist(struct list_head *head)
{
struct list_head *pos, *n;
+ struct fb_modelist *modelist;
+ struct fb_videomode *m;
list_for_each_safe(pos, n, head) {
+ modelist = list_entry(pos, struct fb_modelist, list);
+ m = &modelist->mode;
+ fb_display_clean_videomode(m);
list_del(pos);
kfree(pos);
}
diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
index 81f0e698acbf..2b5e93aeaaff 100644
--- a/include/linux/fbcon.h
+++ b/include/linux/fbcon.h
@@ -18,6 +18,7 @@ void fbcon_suspended(struct fb_info *info);
void fbcon_resumed(struct fb_info *info);
int fbcon_mode_deleted(struct fb_info *info,
struct fb_videomode *mode);
+void fb_display_clean_videomode(struct fb_videomode *m);
void fbcon_new_modelist(struct fb_info *info);
void fbcon_get_requirement(struct fb_info *info,
struct fb_blit_caps *caps);
@@ -38,6 +39,7 @@ static inline void fbcon_suspended(struct fb_info *info) {}
static inline void fbcon_resumed(struct fb_info *info) {}
static inline int fbcon_mode_deleted(struct fb_info *info,
struct fb_videomode *mode) { return 0; }
+static inline void fb_display_clean_videomode(struct fb_videomode *m) {}
static inline void fbcon_new_modelist(struct fb_info *info) {}
static inline void fbcon_get_requirement(struct fb_info *info,
struct fb_blit_caps *caps) {}
--
2.43.0
Hi, thanks for the report. Am 23.09.25 um 13:06 schrieb Quanmin Yan: > Recently, we discovered the following issue through syzkaller: > > BUG: KASAN: slab-use-after-free in fb_mode_is_equal+0x285/0x2f0 > Read of size 4 at addr ff11000001b3c69c by task syz.xxx > ... > Call Trace: > <TASK> > dump_stack_lvl+0xab/0xe0 > print_address_description.constprop.0+0x2c/0x390 > print_report+0xb9/0x280 > kasan_report+0xb8/0xf0 > fb_mode_is_equal+0x285/0x2f0 > fbcon_mode_deleted+0x129/0x180 > fb_set_var+0xe7f/0x11d0 > do_fb_ioctl+0x6a0/0x750 > fb_ioctl+0xe0/0x140 > __x64_sys_ioctl+0x193/0x210 > do_syscall_64+0x5f/0x9c0 > entry_SYSCALL_64_after_hwframe+0x76/0x7e > > The issue occurs in the function fb_mode_is_equal(p->mode, mode), I also > noticed that when freeing the memory related to fb_info->modelist, there's > no attempt to set the corresponding fb_display[i]->mode to NULL after > freeing. Based on analysis, the root cause of this bug appears to be that > a certain p->mode has become a wild pointer. > > I've identified two code paths for freeing modelist->mode: > 1. fb_delete_videomode - removes videomode entry from modelist. > 2. fb_destroy_modelist - destroys the entire modelist. What about fb_new_modelist()? [1] It's called from store_modes() and the whole logic in the caller seems fragile to me. This could leave p->mode set when it should be cleared; and vice versa. [1] https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbmem.c#L712 [2] https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbsysfs.c#L113 > > Analysis shows that fb_delete_videomode path should have been fixed in > a previous patch[1]. Therefore, the current bug is likely triggered > through the fb_destroy_modelist path. I've found a reproducible test case: > 1. With /dev/fb0 already registered in the system, load a kernel module > to register a new device /dev/fb1; > 2. Set fb1's mode to the global fb_display[] array (via FBIOPUT_CON2FBMAP); > 3. Switch console from fb to VGA (to allow normal rmmod of the ko); > 4. Unload the kernel module - at this point fb1's modelist is freed, leaving > a wild pointer in fb_display[]; > 5. Trigger the bug via system calls through fb0 attempting to delete a mode > from fb0. > > To prevent similar issues from recurring, consider traversing fb_display[] > whenever releasing a mode from fb_info. If the corresponding mode exists > in fb_display[], set its pointer to NULL. > > [1] https://lore.kernel.org/all/20210712085544.2828-1-thunder.leizhen@huawei.com/ > > Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com> > --- > This is my first time working on fb issues. If there are any misunderstandings > in my analysis, I would appreciate corrections from the community. > > drivers/video/fbdev/core/fbcon.c | 11 +++++++++++ > drivers/video/fbdev/core/modedb.c | 7 +++++++ > include/linux/fbcon.h | 2 ++ > 3 files changed, 20 insertions(+) > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index b062b05f4128..bfbf79d6cd05 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -2803,6 +2803,17 @@ int fbcon_mode_deleted(struct fb_info *info, > return found; > } > > +void fb_display_clean_videomode(struct fb_videomode *m) Rather fbcon_delete_mode > +{ > + struct fbcon_display *p; > + > + for (int i = first_fb_vc; i <= last_fb_vc; i++) { I think this code also needs to test the fb_info, or it might clear another display's mode. See [3] for an example [3] https://elixir.bootlin.com/linux/v6.16.8/source/drivers/video/fbdev/core/fbcon.c#L2786 > + p = &fb_display[i]; > + if (p->mode == m) > + p->mode = NULL; > + } > +} > + > #ifdef CONFIG_VT_HW_CONSOLE_BINDING > static void fbcon_unbind(void) > { > diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c > index 53a610948c4a..5a0ee96ebefa 100644 > --- a/drivers/video/fbdev/core/modedb.c > +++ b/drivers/video/fbdev/core/modedb.c > @@ -16,6 +16,7 @@ > #include <linux/slab.h> > #include <linux/fb.h> > #include <linux/kernel.h> > +#include <linux/fbcon.h> > > #undef DEBUG > > @@ -1100,6 +1101,7 @@ void fb_delete_videomode(const struct fb_videomode *mode, > modelist = list_entry(pos, struct fb_modelist, list); > m = &modelist->mode; > if (fb_mode_is_equal(m, mode)) { > + fb_display_clean_videomode(m); There's only one caller of fb_delete_videomode(). I think this call should be right before fb_delete_videomode(). > list_del(pos); > kfree(pos); > } > @@ -1113,8 +1115,13 @@ void fb_delete_videomode(const struct fb_videomode *mode, > void fb_destroy_modelist(struct list_head *head) > { > struct list_head *pos, *n; > + struct fb_modelist *modelist; > + struct fb_videomode *m; > > list_for_each_safe(pos, n, head) { > + modelist = list_entry(pos, struct fb_modelist, list); > + m = &modelist->mode; > + fb_display_clean_videomode(m); Same here: I think fb_destroy_modelist() should only release the mode. Clearing the fbcon mode should be done by the caller. Best regards Thomas > list_del(pos); > kfree(pos); > } > diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h > index 81f0e698acbf..2b5e93aeaaff 100644 > --- a/include/linux/fbcon.h > +++ b/include/linux/fbcon.h > @@ -18,6 +18,7 @@ void fbcon_suspended(struct fb_info *info); > void fbcon_resumed(struct fb_info *info); > int fbcon_mode_deleted(struct fb_info *info, > struct fb_videomode *mode); > +void fb_display_clean_videomode(struct fb_videomode *m); > void fbcon_new_modelist(struct fb_info *info); > void fbcon_get_requirement(struct fb_info *info, > struct fb_blit_caps *caps); > @@ -38,6 +39,7 @@ static inline void fbcon_suspended(struct fb_info *info) {} > static inline void fbcon_resumed(struct fb_info *info) {} > static inline int fbcon_mode_deleted(struct fb_info *info, > struct fb_videomode *mode) { return 0; } > +static inline void fb_display_clean_videomode(struct fb_videomode *m) {} > static inline void fbcon_new_modelist(struct fb_info *info) {} > static inline void fbcon_get_requirement(struct fb_info *info, > struct fb_blit_caps *caps) {} -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
© 2016 - 2025 Red Hat, Inc.