| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <234d94ac-3984-4ff9-9743-9178f68370de@suse.de>
Date: Wed, 24 Sep 2025 11:17:56 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Quanmin Yan <yanquanmin1@...wei.com>, simona@...ll.ch
Cc: deller@....de, linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
dri-devel@...ts.freedesktop.org, wangkefeng.wang@...wei.com,
zuoze1@...wei.com, sunnanyong@...wei.com
Subject: Re: [PATCH] fbcon: Set fb_display[i]->mode to NULL when the mode is
released
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@...wei.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)
Powered by blists - more mailing lists