libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.3k stars 1.84k forks source link

Load Core crashes with some subsystem cores. #7997

Open orbea opened 5 years ago

orbea commented 5 years ago

Description

When running a core that supports subsystems like snes9x or sameboy and then loading another core that also supports subsystems it will crash.

So far I have found it will only crash if the second core is fbalpha or bsnes.

Expected behavior

It should not segfault.

Actual behavior

Thread 1 "retroarch" received signal SIGSEGV, Segmentation fault.
0x00007ffff47cc736 in __strlen_sse2 () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff47cc736 in __strlen_sse2 () from /lib64/libc.so.6
#1  0x00007ffff47783ad in vfprintf () from /lib64/libc.so.6
#2  0x00007ffff47a2e29 in vsnprintf () from /lib64/libc.so.6
#3  0x00007ffff477f992 in snprintf () from /lib64/libc.so.6
#4  0x00000000005dea93 in menu_subsystem_populate (subsystem=0x12cdf80,
    info=0x7fffffffdba0) at menu/menu_driver.c:2707
#5  0x00000000005cc43b in xmb_list_push (data=0x13cd260, userdata=0x13d3690,
    info=0x7fffffffdba0, type=7) at menu/drivers/xmb.c:5583
#6  0x00000000005dd599 in menu_driver_push_list (disp_list=0x7fffffffd740)
    at menu/menu_driver.c:2134
#7  0x000000000063b281 in menu_displaylist_ctl (type=DISPLAYLIST_MAIN_MENU,
    info=0x7fffffffdba0) at menu/menu_displaylist.c:4331
#8  0x0000000000615a9d in deferred_push_dlist (info=0x7fffffffdba0,
    state=DISPLAYLIST_MAIN_MENU) at menu/cbs/menu_cbs_deferred_push.c:55
#9  0x000000000061617d in deferred_main_menu_list (info=0x7fffffffdba0)
    at menu/cbs/menu_cbs_deferred_push.c:131
#10 0x000000000063aaf4 in menu_displaylist_push (entry=0x7fffffffdc40)
    at menu/menu_displaylist.c:4075
#11 0x0000000000611e9a in action_refresh_default (list=0x13d8150,
    menu_list=0x13d8130) at menu/cbs/menu_cbs_refresh.c:34
#12 0x000000000060060a in menu_entry_action (entry=0x7fffffffdcd0, i=2,
    action=MENU_ACTION_OK) at menu/widgets/menu_entry.c:502
#13 0x000000000064b967 in generic_menu_iterate (menu=0x13cd260,
    userdata=0x13d3690, action=MENU_ACTION_OK) at menu/drivers/menu_generic.c:232
#14 0x00000000005dd140 in menu_driver_iterate (iterate=0x7fffffffde90)
    at menu/menu_driver.c:2011
#15 0x000000000041858b in runloop_check_state (settings=0x7fffefe3d010,
    input_nonblock_state=false, runloop_is_paused=false, fastforward_ratio=5,
    sleep_ms=0x7fffffffe0f0) at retroarch.c:2850
#16 0x00000000004198c0 in runloop_iterate (sleep_ms=0x7fffffffe0f0)
    at retroarch.c:3563
#17 0x0000000000412717 in rarch_main (argc=1, argv=0x7fffffffe208, data=0x0)
    at frontend/frontend.c:154
#18 0x0000000000412774 in main (argc=1, argv=0x7fffffffe208)
    at frontend/frontend.c:182

Full GDB log - https://pastebin.com/mstV19C8

=================================================================
==3057==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030000d6f80 at pc 0x0000007c062f bp 0x7ffc168e7e30 sp 0x7ffc168e7e28
READ of size 8 at 0x6030000d6f80 thread T0
    #0 0x7c062e in menu_subsystem_populate menu/menu_driver.c:2707
    #1 0x79077e in xmb_list_push menu/drivers/xmb.c:5583
    #2 0x7bd46a in menu_driver_push_list menu/menu_driver.c:2134
    #3 0x87c87c in menu_displaylist_ctl menu/menu_displaylist.c:4331
    #4 0x8338e7 in deferred_push_dlist menu/cbs/menu_cbs_deferred_push.c:55
    #5 0x8340ca in deferred_main_menu_list menu/cbs/menu_cbs_deferred_push.c:131
    #6 0x87aa04 in menu_displaylist_push menu/menu_displaylist.c:4075
    #7 0x82b574 in action_refresh_default menu/cbs/menu_cbs_refresh.c:34
    #8 0x804399 in menu_entry_action menu/widgets/menu_entry.c:502
    #9 0x89f646 in generic_menu_iterate menu/drivers/menu_generic.c:232
    #10 0x7bcae6 in menu_driver_iterate menu/menu_driver.c:2011
    #11 0x42504e in runloop_check_state /home/orbea/gittings/forks/RetroArch/retroarch.c:2850
    #12 0x427493 in runloop_iterate /home/orbea/gittings/forks/RetroArch/retroarch.c:3563
    #13 0x41a5fd in rarch_main frontend/frontend.c:154
    #14 0x41a733 in main frontend/frontend.c:182
    #15 0x7f4fe2d72c66 in __libc_start_main (/lib64/libc.so.6+0x22c66)
    #16 0x40fd59 in _start (/media/gittings/forks/RetroArch/retroarch+0x40fd59)

0x6030000d6f80 is located 0 bytes to the right of 32-byte region [0x6030000d6f60,0x6030000d6f80)
allocated by thread T0 here:
    #0 0x7f4fe6768f30 in malloc (/usr/lib64/libasan.so.5+0xe8f30)
    #1 0x54f286 in rarch_environment_cb /home/orbea/gittings/forks/RetroArch/dynamic.c:1722
    #2 0x7f4fd43342b7 in retro_set_environment (/usr/lib64/libretro/snes9x_libretro.so+0x30c2b7)
    #3 0x42fed9 in command_event_init_core /home/orbea/gittings/forks/RetroArch/command.c:1349
    #4 0x4333cf in command_event /home/orbea/gittings/forks/RetroArch/command.c:2369
    #5 0x42106a in retroarch_main_init /home/orbea/gittings/forks/RetroArch/retroarch.c:1383
    #6 0x448fb4 in content_load tasks/task_content.c:282
    #7 0x44bf30 in task_load_content tasks/task_content.c:884
    #8 0x44c651 in command_event_cmd_exec tasks/task_content.c:1001
    #9 0x44df16 in task_push_load_content_from_playlist_from_menu tasks/task_content.c:1219
    #10 0x80c4f0 in default_action_ok_load_content_from_playlist_from_menu menu/cbs/menu_cbs_ok.c:1546
    #11 0x80d903 in action_ok_playlist_entry_collection menu/cbs/menu_cbs_ok.c:1758
    #12 0x803c75 in menu_entry_action menu/widgets/menu_entry.c:455
    #13 0x89f646 in generic_menu_iterate menu/drivers/menu_generic.c:232
    #14 0x7bcae6 in menu_driver_iterate menu/menu_driver.c:2011
    #15 0x42504e in runloop_check_state /home/orbea/gittings/forks/RetroArch/retroarch.c:2850
    #16 0x427493 in runloop_iterate /home/orbea/gittings/forks/RetroArch/retroarch.c:3563
    #17 0x41a5fd in rarch_main frontend/frontend.c:154
    #18 0x41a733 in main frontend/frontend.c:182
    #19 0x7f4fe2d72c66 in __libc_start_main (/lib64/libc.so.6+0x22c66)

SUMMARY: AddressSanitizer: heap-buffer-overflow menu/menu_driver.c:2707 in menu_subsystem_populate
Shadow bytes around the buggy address:
  0x0c0680012da0: 00 00 fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012db0: 00 00 00 00 fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012dc0: fa fa 00 00 00 00 fa fa fa fa fa fa fa fa fa fa
  0x0c0680012dd0: fa fa fa fa 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c0680012de0: fa fa fa fa fa fa 00 00 00 00 fa fa 00 00 00 00
=>0x0c0680012df0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012e30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680012e40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3057==ABORTING

Steps to reproduce the bug

  1. Start any content with snes9x or sameboy.
  2. Load fbalpha or bsnes.
  3. Segfault.

Bisect Results

45228d030718deed3ecc0eef72f7bef09221cf1f is the first bad commit
commit 45228d030718deed3ecc0eef72f7bef09221cf1f
Author: radius <andres.430@gmail.com>
Date:   Mon Dec 10 23:01:21 2018 -0500

    massive subsystem cleanup & use the proper data in each instance

:040000 040000 7954ef080f12e4449335c94040975cc1eb25e59d 814fefa76bf4faa468038e544b2d40d35edffa71 M  menu

45228d030718deed3ecc0eef72f7bef09221cf1f

Version/Commit

You can find this information under Information/System Information

Environment information

ghost commented 5 years ago

Paging @fr500

orbea commented 5 years ago

It will not crash if the first core is fbalpha and the second core is bsnes, but it will crash if the first core is bsnes and the second core is fbalpha. As fr500 said in irc last night the reason might be that the second core has more subsystems.

andres-asm commented 5 years ago

I am pretty sure I can patch this out but this is really problematic for a number of reasons What is happening is a new core was peeked that has more subsystems than the current one.

So there are two different instances of subsystem data in memory at the same time

The menu uses the following to figure out what to show:

               /* Core fully loaded, use the subsystem data */
               if (system->subsystem.data)
                     subsystem = system->subsystem.data;
               /* Core not loaded completely, use the data we peeked on load core */
               else
                  subsystem = subsystem_data;

               menu_subsystem_populate(subsystem, info);

So, since the first core is STILL FULLY LOADED it's going for the true codepath. Now I could track that a core has changed and force the correct codepath but this is basically what I have been asking forever

LOAD CORE should not be available while a game is loaded. Reasons:

  1. This issue
  2. Saving a remap, if you save a remap after loading another core while a core is active it will be saved incorrectly (it will be saved like core2.rmp)
  3. Same for shader presets
  4. Same for overrides
  5. It's the reason the BIOS check isn't reliable
  6. Maybe even in case of some SAVES it could make them go to the wrong folders
  7. Start netplay, you'll see the wrong core is reported

As you can see, it's a BIG PROBLEM. As I said I think I can pretty much patch this up but it doesn't solve the big problem.

The solutions are:

  1. Hide Load Core when content is active
  2. Make Load Core trigger a close content action when a new core is loaded

Loading a Core while Content is active results in RA being in a really really really inconsistent state. This should be fixed at the root of the problem.

orbea commented 5 years ago

Hide Load Core when content is active

I am okay with this solution and prefer it over the alternative. However I think refactoring the code base to prevent these kinds of issues would be better, but its also much more work...

inactive123 commented 5 years ago

Does this still happen?

orbea commented 5 years ago

Yes, I don't think anything was done beyond discussing if its better to fix this by hiding Load Core when content is loaded or if someone wants to spend time doing a more involved and harder fix.

jdgleaver commented 5 years ago

LOAD CORE should not be available while a game is loaded.

PR #8194 does this for RGUI. Would you like me to do it for the other menu drivers as well...?