search

libretro / mame2003-plus-libretro

Updated 2018 version of MAME (0.78) for libretro. with added game support plus many fixes and improvements
Other
183 stars 109 forks source link

ASAN error on wwfmania #1725

Closed wn2000 closed 2 months ago

wn2000 commented 2 months ago

RK3328 arm32 Linux, built with gcc 11.4.

ASAN log:

==21951==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf2b3680c at pc 0xeb088104 bp 0xffd868bc sp 0xffd868ac
 READ of size 4 at 0xf2b3680c thread T0
     #0 0xeb088100 in pix_convert_palto565 src/mame2003/video.c:275
     #1 0xeb088100 in frame_convert src/mame2003/video.c:364
     #2 0xeb088100 in osd_update_video_and_audio src/mame2003/video.c:478
     #3 0xeb0951d4 in artwork_update_video_and_audio src/artwork.c:808
     #4 0xeb217a70 in update_video_and_audio src/mame.c:1174
     #5 0xeb217c10 in updatescreen src/mame.c:1210
     #6 0xeb0fcc28 in cpu_vblankcallback src/cpuexec.c:1429
     #7 0xeb24d24c in timer_adjust_global_time src/timer.c:327
     #8 0xeb0fe97c in cpu_timeslice src/cpuexec.c:659
     #9 0xeb0fe97c in mame_frame src/cpuexec.c:363
     #10 0xeb079984 in retro_run src/mame2003/mame2003.c:390

0xf2b3680c is located 4 bytes to the right of 131080-byte region [0xf2b16800,0xf2b36808)
 allocated by thread T0 here:
     #0 0xf6b84904 in malloc (libasan.so.6+0xd2904)
     #1 0xeb0c8b60 in auto_malloc src/common.c:941
     #2 0xeb2257f0 in palette_alloc src/palette.c:514
     #3 0xeb2257f0 in palette_start src/palette.c:185
     #4 0xeb215ac8 in vh_open src/mame.c:598
     #5 0xeb215ac8 in run_machine src/mame.c:417
     #6 0xeb215ac8 in run_game src/mame.c:289
     #7 0xeb07902c in retro_load_game src/mame2003/mame2003.c:335

 SUMMARY: AddressSanitizer: heap-buffer-overflow src/mame2003/video.c:275 in pix_convert_palto565
 Shadow bytes around the buggy address:
  0x3e566cb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e566cc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e566cd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e566ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e566cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x3e566d00: 00[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e566d10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e566d20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e566d30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e566d40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e566d50: 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
  Shadow gap:              cc
==21951==ABORTING
mahoneyt944 commented 2 months ago

See if fixing the casting helps between uint16_t and uint32_t

wn2000 commented 2 months ago

Hmmm so the pallete has 32770 colors it seems. It's as if some pixel has a color out of that range.

mahoneyt944 commented 2 months ago

I've seen instances like that before, could try size_t.

mahoneyt944 commented 2 months ago

While referencing mame2010, I found this. Seems like this is a known and accepted loss in the conversion?

    // the accumulation buffer is always 8888
    //
    // the standard format for the framebuffer appears to be 565
    // yes, this means colour data is lost in the conversion
wn2000 commented 2 months ago

While referencing mame2010, I found this. Seems like this is a known and accepted loss in the conversion?

  // the accumulation buffer is always 8888
  //
  // the standard format for the framebuffer appears to be 565
  // yes, this means colour data is lost in the conversion

You are mixing up the concepts of color value and color index.

The palette in this case has 32770 (32768 + 2) color indices. Each color index corresponds to a full 8888 color value.

The process seems to be: For each pixel in the frame, get its color index, then look up the pallete to get the 8888 color, then convert that to 565 color (with an acceptable loss of fidelity).

The issue identified by ASAN, is that one pixel has a color index of 32771, when it should be within [0, 32769].

mahoneyt944 commented 2 months ago

we could raise the palette length. Though it doesn't explain the extra 2.

https://github.com/libretro/mame2003-plus-libretro/blob/2154d31e620a016685c4d8c155a4d6a7000927a1/src/drivers/midwunit.c#L615