Page MenuHome

heap use after free at clicking on splash screen's open button
Closed, ResolvedPublicBUG

Description

macOS 10.14
Intel HD 6000

Broken: rBbb872b25f219d1a9bc2446228b6dc7a9248d7f20

Steps to redo:

  • click the open button on the splash screen

Asan report:

==51777==ERROR: AddressSanitizer: heap-use-after-free on address 0x61400009ed20 at pc 0x0001001db693 bp 0x7ffeefbfc9d0 sp 0x7ffeefbfc9c8
READ of size 2 at 0x61400009ed20 thread T0
    #0   ED_region_tag_refresh_ui source/blender/editors/screen/area.c:678:21
    #1   wm_block_splash_refreshmenu source/blender/windowmanager/intern/wm_splash_screen.c:78:3
    #2   ui_apply_but_funcs_after source/blender/editors/interface/interface_handlers.c:948:7
    #3   ui_popup_handler source/blender/editors/interface/interface_handlers.c:10861:3
    #4   wm_handler_ui_call source/blender/windowmanager/intern/wm_event_system.c:640:12
    #5   wm_handlers_do_intern source/blender/windowmanager/intern/wm_event_system.c:2756:21
    #6   wm_handlers_do source/blender/windowmanager/intern/wm_event_system.c:2867:16
    #7   wm_event_do_handlers source/blender/windowmanager/intern/wm_event_system.c:3291:17
    #8   WM_main source/blender/windowmanager/intern/wm.c:481:5
    #9   main source/creator/creator.c:519:5
    #10 0x7fff715413d4 in start (/usr/lib/system/libdyld.dylib:x86_64+0x163d4)

0x61400009ed20 is located 224 bytes inside of 424-byte region [0x61400009ec40,0x61400009ede8)
freed by thread T0 here:
    #0 0x111e49576 in wrap_free (/Users/ankitkumar/Applications/usr/lib/clang/12.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x46576)
    #1   MEM_lockfree_freeN intern/guardedalloc/intern/mallocn_lockfree_impl.c:129:5
    #2   BLI_freelinkN source/blender/blenlib/intern/listbase.c:290:3
    #3   ui_region_temp_remove source/blender/editors/interface/interface_regions.c:67:3
    #4   ui_popup_block_remove source/blender/editors/interface/interface_region_popup.c:538:3
    #5   ui_popup_block_free source/blender/editors/interface/interface_region_popup.c:851:3
    #6   ui_popup_handler_remove source/blender/editors/interface/interface_handlers.c:10890:3
    #7   WM_event_free_ui_handler_all source/blender/windowmanager/intern/wm_event_system.c:3904:9
    #8   UI_popup_handlers_remove_all source/blender/editors/interface/interface_handlers.c:10936:3
    #9   WM_event_add_fileselect source/blender/windowmanager/intern/wm_event_system.c:3484:3
    #10   wm_open_mainfile__select_file_path source/blender/windowmanager/intern/wm_files.c:2305:3
    #11   operator_state_dispatch source/blender/windowmanager/intern/wm_files.c:2231:14
    #12   wm_open_mainfile_dispatch source/blender/windowmanager/intern/wm_files.c:2359:10
    #13   wm_open_mainfile__discard_changes source/blender/windowmanager/intern/wm_files.c:2276:10
    #14   operator_state_dispatch source/blender/windowmanager/intern/wm_files.c:2231:14
    #15   wm_open_mainfile_dispatch source/blender/windowmanager/intern/wm_files.c:2359:10
    #16   wm_open_mainfile_invoke source/blender/windowmanager/intern/wm_files.c:2364:10
    #17   wm_operator_invoke source/blender/windowmanager/intern/wm_event_system.c:1303:16
    #18   wm_operator_call_internal source/blender/windowmanager/intern/wm_event_system.c:1549:16
    #19   WM_operator_name_call_ptr source/blender/windowmanager/intern/wm_event_system.c:1563:10
    #20   ui_apply_but_funcs_after source/blender/editors/interface/interface_handlers.c:931:7
    #21   ui_popup_handler source/blender/editors/interface/interface_handlers.c:10861:3
    #22   wm_handler_ui_call source/blender/windowmanager/intern/wm_event_system.c:640:12
    #23   wm_handlers_do_intern source/blender/windowmanager/intern/wm_event_system.c:2756:21
    #24   wm_handlers_do source/blender/windowmanager/intern/wm_event_system.c:2867:16
    #25   wm_event_do_handlers source/blender/windowmanager/intern/wm_event_system.c:3291:17
    #26   WM_main source/blender/windowmanager/intern/wm.c:481:5
    #27   main source/creator/creator.c:519:5
    #28  start (/usr/lib/system/libdyld.dylib:x86_64+0x163d4)

previously allocated by thread T0 here:
    #0  wrap_calloc (/Users/me/Applications/usr/lib/clang/12.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x46802)
    #1   MEM_lockfree_callocN intern/guardedalloc/intern/mallocn_lockfree_impl.c:235:21
    #2   ui_region_temp_add source/blender/editors/interface/interface_regions.c:46:12
    #3   ui_popup_block_create source/blender/editors/interface/interface_region_popup.c:810:12
    #4   UI_popup_block_invoke_ex source/blender/editors/interface/interface_region_menu_popup.c:588:12
    #5   UI_popup_block_invoke source/blender/editors/interface/interface_region_menu_popup.c:605:3
    #6   wm_splash_invoke source/blender/windowmanager/intern/wm_splash_screen.c:248:3
    #7   wm_operator_invoke source/blender/windowmanager/intern/wm_event_system.c:1303:16
    #8   wm_operator_call_internal source/blender/windowmanager/intern/wm_event_system.c:1549:16
    #9   WM_operator_name_call_ptr source/blender/windowmanager/intern/wm_event_system.c:1563:10
    #10   WM_operator_name_call source/blender/windowmanager/intern/wm_event_system.c:1569:12
    #11   WM_init_splash source/blender/windowmanager/intern/wm_init_exit.c:405:7
    #12   main source/creator/creator.c:517:7
    #13 start (/usr/lib/system/libdyld.dylib:x86_64+0x163d4)

Event Timeline

Ankit Meel (ankitm) renamed this task from use after free at clicking on splash screen's open button to heap use after free at clicking on splash screen's open button.Oct 18 2020, 9:45 PM
Ankit Meel (ankitm) created this task.
Ankit Meel (ankitm) updated the task description. (Show Details)
Hans Goudey (HooglyBoogly) changed the task status from Needs Triage to Confirmed.Oct 18 2020, 10:19 PM

Not sure if this sort of thing is triaged as a bug? But I can reproduce this. Master from July 15th also had the same problem, so I wouldn't guess that this is a recently introduced issue.

Why would this be a thread-race? I don't see a related part here that is multi-threaded. To me this seems like a regular use-after-free that should be easy to fix.

@Julian Eisel (Severin) Seems like I need some glasses. I misread the ASAN report and was under the impression that the first and second trace were from different threads, which is obviously not true. Please disregard my previous comment.

This comment was removed by Robert Guetzkow (rjg).
Julian Eisel (Severin) triaged this task as High priority.Oct 26 2020, 8:11 PM
Julian Eisel (Severin) changed the subtype of this task from "Report" to "Bug".

Not sure if this is a 2.91 regression, but let's treat it as one until we know better.

@Hans Goudey (HooglyBoogly) what is your OS? can you reproduce it in 2.90 release?

I'm using Fedora 32, kernel 5.8. Although I doubt this bug depends on the OS. And yes, I can reproduce this on blender-v2.90-release.

How about just doing this:

diff --git a/source/blender/windowmanager/intern/wm_splash_screen.c b/source/blender/windowmanager/intern/wm_splash_screen.c
index ec1c4440474..d732393b631 100644
--- a/source/blender/windowmanager/intern/wm_splash_screen.c
+++ b/source/blender/windowmanager/intern/wm_splash_screen.c
@@ -72,12 +72,6 @@ static void wm_block_close(bContext *C, void *arg_block, void *UNUSED(arg))
   UI_popup_block_close(C, win, arg_block);
 }
 
-static void wm_block_splash_refreshmenu(bContext *C, void *UNUSED(arg_block), void *UNUSED(arg))
-{
-  ARegion *region_menu = CTX_wm_menu(C);
-  ED_region_tag_refresh_ui(region_menu);
-}
-
 static void wm_block_splash_add_label(uiBlock *block, const char *label, int x, int y)
 {
   if (!(label && label[0])) {
@@ -217,7 +211,6 @@ static uiBlock *wm_block_create_splash(bContext *C, ARegion *region, void *UNUSE
       block, ibuf, 0, 0.5f * U.widget_unit, splash_width, splash_height, NULL);
 
   UI_but_func_set(but, wm_block_close, block, NULL);
-  UI_block_func_set(block, wm_block_splash_refreshmenu, block, NULL);
 
   wm_block_splash_add_label(
       block, BKE_blender_version_string(), splash_width, splash_height - 13.0 * U.dpi_fac);

I don't really know what UI_block_func_set is, but in my basic test it does not seem necessary.

For context, the wm_block_splash_refreshmenu function was added in rB486796ce31cd: * Interaction Presets.

I checked this and it seems fine. I can't see errors without this explicit refreshing, back then popup refreshing wasn't as "sophisticated" as it is today.

To a degree this is just hiding the issue but I think that's okay still. A proper fix should be easy but could also backfire.