Skip to content

Commit a1f3058

Browse files
Quanmin Yanhdeller
authored andcommitted
fbcon: Set fb_display[i]->mode to NULL when the mode is released
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 Based on experimentation and analysis, during framebuffer unregistration, only the memory of fb_info->modelist is freed, without setting the corresponding fb_display[i]->mode to NULL for the freed modes. This leads to UAF issues during subsequent accesses. Here's an example of reproduction steps: 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. Add a check in do_unregister_framebuffer(): if the mode to be freed exists in fb_display[], set the corresponding mode pointer to NULL. Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Signed-off-by: Helge Deller <deller@gmx.de> Cc: stable@vger.kernel.org
1 parent 18c4ef4 commit a1f3058

File tree

3 files changed

+22
-0
lines changed

3 files changed

+22
-0
lines changed

drivers/video/fbdev/core/fbcon.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,6 +2810,25 @@ int fbcon_mode_deleted(struct fb_info *info,
28102810
return found;
28112811
}
28122812

2813+
static void fbcon_delete_mode(struct fb_videomode *m)
2814+
{
2815+
struct fbcon_display *p;
2816+
2817+
for (int i = first_fb_vc; i <= last_fb_vc; i++) {
2818+
p = &fb_display[i];
2819+
if (p->mode == m)
2820+
p->mode = NULL;
2821+
}
2822+
}
2823+
2824+
void fbcon_delete_modelist(struct list_head *head)
2825+
{
2826+
struct fb_modelist *modelist;
2827+
2828+
list_for_each_entry(modelist, head, list)
2829+
fbcon_delete_mode(&modelist->mode);
2830+
}
2831+
28132832
#ifdef CONFIG_VT_HW_CONSOLE_BINDING
28142833
static void fbcon_unbind(void)
28152834
{

drivers/video/fbdev/core/fbmem.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,7 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
544544
fb_info->pixmap.addr = NULL;
545545
}
546546

547+
fbcon_delete_modelist(&fb_info->modelist);
547548
fb_destroy_modelist(&fb_info->modelist);
548549
registered_fb[fb_info->node] = NULL;
549550
num_registered_fb--;

include/linux/fbcon.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ void fbcon_suspended(struct fb_info *info);
1818
void fbcon_resumed(struct fb_info *info);
1919
int fbcon_mode_deleted(struct fb_info *info,
2020
struct fb_videomode *mode);
21+
void fbcon_delete_modelist(struct list_head *head);
2122
void fbcon_new_modelist(struct fb_info *info);
2223
void fbcon_get_requirement(struct fb_info *info,
2324
struct fb_blit_caps *caps);
@@ -38,6 +39,7 @@ static inline void fbcon_suspended(struct fb_info *info) {}
3839
static inline void fbcon_resumed(struct fb_info *info) {}
3940
static inline int fbcon_mode_deleted(struct fb_info *info,
4041
struct fb_videomode *mode) { return 0; }
42+
static inline void fbcon_delete_modelist(struct list_head *head) {}
4143
static inline void fbcon_new_modelist(struct fb_info *info) {}
4244
static inline void fbcon_get_requirement(struct fb_info *info,
4345
struct fb_blit_caps *caps) {}

0 commit comments

Comments
 (0)