[PATCH 2/2] fbdev: fbmem: Fix double free of 'fb_info->pixmap.addr'

[PATCH 2/2] fbdev: fbmem: Fix double free of 'fb_info->pixmap.addr'

[Zheyu Ma]

savagefb and some other drivers call kfree to free 'info->pixmap.addr'
even after calling unregister_framebuffer, which may cause double free.

Fix this by setting 'fb_info->pixmap.addr' to NULL after kfree in
unregister_framebuffer.

The following log reveals it:

[   37.318872] BUG: KASAN: double-free or invalid-free in kfree+0x13e/0x290
[   37.319369]
[   37.320803] Call Trace:
[   37.320992]  dump_stack_lvl+0xa8/0xd1
[   37.321274]  print_address_description+0x87/0x3b0
[   37.321632]  ? kfree+0x13e/0x290
[   37.321879]  ? kfree+0x13e/0x290
[   37.322126]  ? kfree+0x13e/0x290
[   37.322374]  kasan_report_invalid_free+0x58/0x90
[   37.322724]  ____kasan_slab_free+0x123/0x140
[   37.323049]  __kasan_slab_free+0x11/0x20
[   37.323347]  slab_free_freelist_hook+0x81/0x150
[   37.323689]  ? savagefb_remove+0xa1/0xc0 [savagefb]
[   37.324066]  kfree+0x13e/0x290
[   37.324304]  savagefb_remove+0xa1/0xc0 [savagefb]
[   37.324655]  pci_device_remove+0xa9/0x250
[   37.324959]  ? pci_device_probe+0x7d0/0x7d0
[   37.325273]  device_release_driver_internal+0x4f7/0x7a0
[   37.325666]  driver_detach+0x1e8/0x2c0
[   37.325952]  bus_remove_driver+0x134/0x290
[   37.326262]  ? sysfs_remove_groups+0x97/0xb0
[   37.326584]  driver_unregister+0x77/0xa0
[   37.326883]  pci_unregister_driver+0x2c/0x1c0
[   37.336124]
[   37.336245] Allocated by task 5465:
[   37.336507]  ____kasan_kmalloc+0xb5/0xe0
[   37.336801]  __kasan_kmalloc+0x9/0x10
[   37.337069]  kmem_cache_alloc_trace+0x12b/0x220
[   37.337405]  register_framebuffer+0x3f3/0xa00
[   37.337731]  foo_register_framebuffer+0x3b/0x50 [savagefb]
[   37.338136]
[   37.338255] Freed by task 5475:
[   37.338492]  kasan_set_track+0x3d/0x70
[   37.338774]  kasan_set_free_info+0x23/0x40
[   37.339081]  ____kasan_slab_free+0x10b/0x140
[   37.339399]  __kasan_slab_free+0x11/0x20
[   37.339694]  slab_free_freelist_hook+0x81/0x150
[   37.340034]  kfree+0x13e/0x290
[   37.340267]  do_unregister_framebuffer+0x21c/0x3d0
[   37.340624]  unregister_framebuffer+0x23/0x40
[   37.340950]  savagefb_remove+0x45/0xc0 [savagefb]
[   37.341302]  pci_device_remove+0xa9/0x250
[   37.341603]  device_release_driver_internal+0x4f7/0x7a0
[   37.341990]  driver_detach+0x1e8/0x2c0
[   37.342272]  bus_remove_driver+0x134/0x290
[   37.342577]  driver_unregister+0x77/0xa0
[   37.342873]  pci_unregister_driver+0x2c/0x1c0
[   37.343196]  cleanup_module+0x15/0x1c [savagefb]
[   37.343543]  __se_sys_delete_module+0x398/0x490
[   37.343881]  __x64_sys_delete_module+0x56/0x60
[   37.344221]  do_syscall_64+0x4d/0xc0
[   37.344492]  entry_SYSCALL_64_after_hwframe+0x44/0xae

Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
---
 drivers/video/fbdev/core/fbmem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 7420d2c16e47..b81b0e37daed 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1702,8 +1702,10 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
 {
 	unlink_framebuffer(fb_info);
 	if (fb_info->pixmap.addr &&
-	    (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
+	    (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT)) {
 		kfree(fb_info->pixmap.addr);
+		fb_info->pixmap.addr = NULL;
+	}
 	fb_destroy_modelist(&fb_info->modelist);
 	registered_fb[fb_info->node] = NULL;
 	num_registered_fb--;
-- 
2.17.6

Re: [PATCH 2/2] fbdev: fbmem: Fix double free of 'fb_info->pixmap.addr'

[Sam Ravnborg]

Hi Zheyu,

On Sun, Oct 10, 2021 at 06:42:28AM +0000, Zheyu Ma wrote:
> savagefb and some other drivers call kfree to free 'info->pixmap.addr'
> even after calling unregister_framebuffer, which may cause double free.
> 
> Fix this by setting 'fb_info->pixmap.addr' to NULL after kfree in
> unregister_framebuffer.

This is a nice general way to fix it that benefits all drivers.
Applied to drm-misc-next as this is not an urgent fix.

	Sam