search

libretro / RetroArch

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

Stack overflow when switching language #7508

Closed ghost closed 5 years ago

ghost commented 5 years ago

Description

When running under ASAN on Linux, trying to change the language in certain circumstances produces a stack overflow:

gfx/font_driver.c:770:46: runtime error: left shift of negative value -40
gfx/font_driver.c:789:34: runtime error: left shift of negative value -39
gfx/font_driver.c:819:42: runtime error: left shift of negative value -40
=================================================================
==7514==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd54aba98f at pc 0x55e02de26837 bp 0x7ffd54ab9fe0 sp 0x7ffd54ab9fd0
READ of size 1 at 0x7ffd54aba98f thread T0
    #0 0x55e02de26836 in font_driver_reshape_msg gfx/font_driver.c:849
    #1 0x55e02de27a97 in font_driver_render_msg gfx/font_driver.c:932
    #2 0x55e02e62b74f in gl_set_osd_msg gfx/drivers/gl.c:836
    #3 0x55e02dd1360c in video_driver_set_osd_msg gfx/video_driver.c:1179
    #4 0x55e02e3cdda8 in menu_display_draw_text menu/menu_driver.c:1600
    #5 0x55e02e359620 in xmb_draw_text menu/drivers/xmb.c:812
    #6 0x55e02e36ef51 in xmb_draw_item menu/drivers/xmb.c:2756
    #7 0x55e02e371be6 in xmb_draw_items menu/drivers/xmb.c:2917
    #8 0x55e02e382aec in xmb_frame menu/drivers/xmb.c:3869
    #9 0x55e02e3cf987 in menu_driver_frame menu/menu_driver.c:1884
    #10 0x55e02e632726 in gl_frame gfx/drivers/gl.c:1131
    #11 0x55e02dd1d59b in video_driver_frame gfx/video_driver.c:2619
    #12 0x55e02dd14eff in video_driver_cached_frame gfx/video_driver.c:1423
    #13 0x55e02e3b5d62 in menu_display_libretro menu/menu_driver.c:541
    #14 0x55e02e3d01ce in menu_driver_render menu/menu_driver.c:1925
    #15 0x55e02db75b62 in runloop_check_state /home/bp/RetroArch/retroarch.c:2789
    #16 0x55e02db794fc in runloop_iterate /home/bp/RetroArch/retroarch.c:3440
    #17 0x55e02df1ceb7 in ui_application_qt_run ui/drivers/qt/ui_qt_application.cpp:158
    #18 0x55e02db628e9 in rarch_main frontend/frontend.c:157
    #19 0x55e02df1d00a in main ui/drivers/qt/ui_qt_application.cpp:182
    #20 0x7ff0cb4bf222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #21 0x55e02db5283d in _start (/home/bp/RetroArch/retroarch+0x3c5983d)

Address 0x7ffd54aba98f is located in stack of thread T0 at offset 287 in frame
    #0 0x55e02e36c1af in xmb_draw_item menu/drivers/xmb.c:2607

  This frame has 6 object(s):
    [32, 64) 'rotate_draw'
    [96, 136) 'ticker'
    [192, 256) 'mymat_tmp'
    [288, 543) 'tmp' <== Memory access at offset 287 underflows this variable
    [576, 1088) 'entry_sublabel'
    [1120, 5216) 'entry_path'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow gfx/font_driver.c:849 in font_driver_reshape_msg
Shadow bytes around the buggy address:
  0x10002a94f4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002a94f4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002a94f500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
  0x10002a94f510: f1 f1 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 f2
  0x10002a94f520: f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 f2 f2
=>0x10002a94f530: f2[f2]00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002a94f540: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002a94f550: 00 07 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00
  0x10002a94f560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002a94f570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10002a94f580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==7514==ABORTING

Happens at least with both xmb and ozone.

Expected behavior

No stack overflow.

Actual behavior

Stack overflow.

Steps to reproduce the bug

It can be triggered a few different ways:

(assuming XMB)

  1. Go to Settings -> User -> Language, press right until just after Vietnamese, it crashes
  2. Go to Settings -> User -> Language, press left until just after Greek, it crashes
  3. Go to Settings -> User -> Language, press enter to show the selection submenu, then press down until just after French, it crashes (seems dependent on the window size... if you resize it to show the entry after Vietnamese (which would be Arabic?), it will crash too)

Bisect Results

Issue is somewhere between 1.7.0 (good) and 1.7.1 (bad), but too many commits had to be skipped during the bisect because they all crashed on startup.

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
1f3e1baeb6eec69870121ad3b9c90b7c20e52528
1bfea60c2d56942659c7ae5f69cf3fcdc9f28454
da0e2e6a51abef60f27ca76387129a0c4c532bc3
31b99e2b01f2b7bc4e5b27c4a50bb15772589d49
a8af4ee8c618644237cc736e8c927c5a43d0d2d9
01d659a4a3b1f7525128bdf41979466242c54b29
843a7f200cb36b0d9c9960394f9fd895466cbe98
40e9416c6ca44f501afbf22aa4facaa8fa505594
b141c37fe924b2b4b987ed254c86f5ccc1181b87
49bea666ff54ac7e11f984b90e702f0a7e145107
b5d2782833373287565e31aa4b9c6133c59bc223
b45b1b3e55f90758fe1d8a6fbdac202b71bb4a45
bfb71f06946de1825a8005d65b2ff5b46aaa9804
875ab9378dbbcd80f0fa4f838b5ed549c33a7f95
b31779c588fbdc1828be42350491ec7786c1d602
72ff4c4d444ff660be79d77bc3b8f124ab769d15
8b1f8b8ae2b85d680c9734bdba5a3942de2b617f
a8ee5f6c44a6253bb4cfd6a8085ef46505cada6e
febfc18f0b118076cab9678ba0e89da7d623e87c
We cannot bisect more!

Version/Commit

Environment information

orbea commented 5 years ago

This might be related. https://github.com/libretro/RetroArch/issues/7822

orbea commented 5 years ago

Updated stacktrace.

$ ./retroarch
=================================================================
==23205==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe39456a7f at pc 0x00000058e7ca bp 0x7ffe39456160 sp 0x7ffe39456158
READ of size 1 at 0x7ffe39456a7f thread T0
    #0 0x58e7c9 in font_driver_reshape_msg gfx/font_driver.c:884
    #1 0x58f2ee in font_driver_render_msg gfx/font_driver.c:967
    #2 0x90e499 in gl_set_osd_msg gfx/drivers/gl.c:833
    #3 0x505568 in video_driver_set_osd_msg gfx/video_driver.c:1184
    #4 0x7ba897 in menu_display_draw_text menu/menu_driver.c:1607
    #5 0x76e9c0 in xmb_draw_text menu/drivers/xmb.c:847
    #6 0x77bdda in xmb_draw_item menu/drivers/xmb.c:2942
    #7 0x77d956 in xmb_draw_items menu/drivers/xmb.c:3109
    #8 0x786a1e in xmb_frame menu/drivers/xmb.c:4080
    #9 0x7bb914 in menu_driver_frame menu/menu_driver.c:1894
    #10 0x911b14 in gl_frame gfx/drivers/gl.c:1121
    #11 0x50b99a in video_driver_frame gfx/video_driver.c:2639
    #12 0x506401 in video_driver_cached_frame gfx/video_driver.c:1428
    #13 0x7ae486 in menu_display_libretro menu/menu_driver.c:548
    #14 0x7bbc94 in menu_driver_render menu/menu_driver.c:1935
    #15 0x425042 in runloop_check_state /home/orbea/gittings/forks/RetroArch/retroarch.c:2867
    #16 0x42737f in runloop_iterate /home/orbea/gittings/forks/RetroArch/retroarch.c:3562
    #17 0x41a5fd in rarch_main frontend/frontend.c:141
    #18 0x41a733 in main frontend/frontend.c:169
    #19 0x7fec5e9eac66 in __libc_start_main (/lib64/libc.so.6+0x22c66)
    #20 0x40fd59 in _start (/media/gittings/forks/RetroArch/retroarch+0x40fd59)

Address 0x7ffe39456a7f is located in stack of thread T0 at offset 287 in frame
    #0 0x77a5fe in xmb_draw_item menu/drivers/xmb.c:2793

  This frame has 6 object(s):
    [32, 64) 'rotate_draw'
    [96, 136) 'ticker'
    [192, 256) 'mymat_tmp'
    [288, 543) 'tmp' <== Memory access at offset 287 underflows this variable
    [576, 1088) 'entry_sublabel'
    [1120, 5216) 'entry_path'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow gfx/font_driver.c:884 in font_driver_reshape_msg
Shadow bytes around the buggy address:
  0x100047282cf0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 f2 f2 f2
  0x100047282d00: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x100047282d10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100047282d20: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x100047282d30: 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 f2 f2 f2
=>0x100047282d40: f2 f2 f2 f2 00 00 00 00 00 00 00 00 f2 f2 f2[f2]
  0x100047282d50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100047282d60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 07
  0x100047282d70: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
  0x100047282d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100047282d90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==23205==ABORTING
orbea commented 5 years ago

I tried to bisect this and got this commit.

a8ee5f6c44a6253bb4cfd6a8085ef46505cada6e is the first bad commit
commit a8ee5f6c44a6253bb4cfd6a8085ef46505cada6e
Author: aliaspider <aliaspider@gmail.com>
Date:   Fri Feb 9 16:59:48 2018 +0100

    restore some changes made in 9dc597cf6c629fae4962031956261d2429395063.

:040000 040000 86e4387e140fc22f16933461682fd26ff26862d7 ce4856621da8de86166908706aa5cd24677cb509 M  menu

a8ee5f6c44a6253bb4cfd6a8085ef46505cada6e

orbea commented 5 years ago

With the bad commit it stops crashing with this change. (Not a fix)

diff --git a/menu/menu_setting.c b/menu/menu_setting.c
index ff952fec2d..e323ff140e 100644
--- a/menu/menu_setting.c
+++ b/menu/menu_setting.c
@@ -605,7 +605,6 @@ static void setting_get_string_representation_uint_user_language(void *data,
    modes[RETRO_LANGUAGE_ESPERANTO]              = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_ESPERANTO);
    modes[RETRO_LANGUAGE_POLISH]                 = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_POLISH);
    modes[RETRO_LANGUAGE_VIETNAMESE]             = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_VIETNAMESE);
-   modes[RETRO_LANGUAGE_ARABIC]                 = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_ARABIC);

    strlcpy(s, modes[*msg_hash_get_uint(MSG_HASH_USER_LANGUAGE)], len);
 }

This however doesn't seem to work with the master.

orbea commented 5 years ago

I bisected again while removing RETRO_LANGUAGE_ARABIC from menu/menu_settings.c before each build and found this commit where it started to crash again.

6338039ac0c469420ed9a25cc3cc980501cc2b22 is the first bad commit
commit 6338039ac0c469420ed9a25cc3cc980501cc2b22
Author: Alfrix <alfredomonclus@gmail.com>
Date:   Mon Oct 15 22:32:03 2018 -0300

    Move Privacy settings to User

:040000 040000 bd104f80cbc6f41c0c2059acb9f9d40268ebe9a9 96ff48dd3674ac00c35f7bd8b892f92a719673cc M  menu

6338039ac0c469420ed9a25cc3cc980501cc2b22

 sed -i '/RETRO_LANGUAGE_ARABIC/d' menu/menu_setting.c
orbea commented 5 years ago

This hack prevents the crashes with the master.

@alfrix and @aliaspider Any help you can provide would be appreciated!

diff --git a/menu/menu_displaylist.c b/menu/menu_displaylist.c
index a891d68701..9f14e76157 100644
--- a/menu/menu_displaylist.c
+++ b/menu/menu_displaylist.c
@@ -6451,9 +6451,6 @@ bool menu_displaylist_ctl(enum menu_displaylist_ctl_state type, menu_displaylist
          break;
       case DISPLAYLIST_USER_SETTINGS_LIST:
          menu_entries_ctl(MENU_ENTRIES_CTL_CLEAR, info->list);
-         menu_displaylist_parse_settings_enum(menu, info,
-               MENU_ENUM_LABEL_PRIVACY_SETTINGS,
-               PARSE_ACTION, false);

          if (menu_displaylist_parse_settings_enum(menu, info,
                MENU_ENUM_LABEL_ACCOUNTS_LIST,
@@ -7086,6 +7083,8 @@ bool menu_displaylist_ctl(enum menu_displaylist_ctl_state type, menu_displaylist
                MENU_ENUM_LABEL_USER_SETTINGS,   PARSE_ACTION, false);
          ret = menu_displaylist_parse_settings_enum(menu, info,
                MENU_ENUM_LABEL_DIRECTORY_SETTINGS,   PARSE_ACTION, false);
+         ret = menu_displaylist_parse_settings_enum(menu, info,
+               MENU_ENUM_LABEL_PRIVACY_SETTINGS,   PARSE_ACTION, false);
          info->need_push    = true;
          break;
       case DISPLAYLIST_HORIZONTAL:
diff --git a/menu/menu_setting.c b/menu/menu_setting.c
index 592008b136..4f9ac396f0 100644
--- a/menu/menu_setting.c
+++ b/menu/menu_setting.c
@@ -1928,7 +1928,6 @@ static void setting_get_string_representation_uint_user_language(
    modes[RETRO_LANGUAGE_ESPERANTO]              = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_ESPERANTO);
    modes[RETRO_LANGUAGE_POLISH]                 = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_POLISH);
    modes[RETRO_LANGUAGE_VIETNAMESE]             = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_VIETNAMESE);
-   modes[RETRO_LANGUAGE_ARABIC]                 = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_ARABIC);
    modes[RETRO_LANGUAGE_GREEK]                 = msg_hash_to_str(MENU_ENUM_LABEL_VALUE_LANG_GREEK);
    strlcpy(s, modes[*msg_hash_get_uint(MSG_HASH_USER_LANGUAGE)], len);
 }