alsa-project / alsa-lib

The Advanced Linux Sound Architecture (ALSA) - library
GNU Lesser General Public License v2.1
360 stars 177 forks source link

Sending 0xFx events causes libasound to segfault #392

Closed human222 closed 6 months ago

human222 commented 6 months ago

ALSA 1.1.9. This issue breaks DOSBox-X MIDI playback.

Discovered through this thread: https://github.com/joncampbell123/dosbox-x/issues/4873

In particular, this comment: https://github.com/joncampbell123/dosbox-x/issues/4873#issuecomment-2026694848

tiwai commented 6 months ago

Could you verify whether the problem persists with the latest alsa-lib release 1.2.11? 1.1.9 was quite old, and there have been significant changes in ALSA sequencer code for MIDI 2.0 support, so it makes little sense to stick with that version for debugging.

If the problem persists, please get the stack trace with debuginfo enabled for alsa-lib.

human222 commented 6 months ago

I'm not familiar with stack trace, but I seem to be having the same issue as jon, on alsa-lib 1.2.11. Is there a quick one-liner I can try?

tiwai commented 6 months ago

If you have distro packages, you likely have the corresponding *-debuginfo package. Once after installing it and catch the crash with gdb, gdb will show the lines of the source code.

tiwai commented 6 months ago

That is, I'd like to get the exact line that crashes in the gdb stack trace shown in https://github.com/joncampbell123/dosbox-x/issues/4873#issuecomment-2026625002

Also, what data was sent to snd_seq_event_output(), i.e. the dump of snd_seq_event_t data that triggers the crash.

tiwai commented 6 months ago

And now I took a look at the code of dosbox-x. It's likely a bug there, not a bug in alsa-lib.

The code sending the event 0xf0 should have snd_seq_ev_set_fixed(&ev) before send_event(1). The problem is that the previous event was a SYSEX that is a variable-length event, and the flag isn't cleared if you re-use the same event record.

tiwai commented 6 months ago

Note that other event types call like snd_seq_ev_set_chanpress() and those macros set the event flag bits properly inside. The buggy case is submitting directly and hits the problem because of the lack of the event flag initialization.

grapeli commented 6 months ago

@tiwai Maybe this debug log will be more helpful in explaining the cause of the error.

AddressSanitizer:DEADLYSIGNAL
=================================================================
==91726==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x70b2444c4401 bp 0x7fffd58ccfe0 sp 0x7fffd58cc788 T0)
==91726==The signal is caused by a READ memory access.
==91726==Hint: address points to the zero page.
    #0 0x70b2444c4401 in memcpy (/usr/lib/libc.so.6+0xa6401) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #1 0x600f250551d2 in memcpy /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115:5
    #2 0x70b245076a9a in snd_seq_event_output_buffer /tmp/alsa-lib/src/seq/seq.c:4201:4
    #3 0x70b2450768b5 in snd_seq_event_output /tmp/alsa-lib/src/seq/seq.c:4154:11
    #4 0x600f277b4644 in MidiHandler_alsa::send_event(int) /tmp/dosbox-x/src/gui/./midi_alsa.h:95:3
    #5 0x600f277b1d1e in MidiHandler_alsa::PlayMsg(unsigned char*) /tmp/dosbox-x/src/gui/./midi_alsa.h:169:4
    #6 0x600f2778933f in MIDI_RawOutByte(unsigned char) /tmp/dosbox-x/src/gui/midi.cpp:520:17
    #7 0x600f2816f1ef in MPU401_WriteData(unsigned long, unsigned long, unsigned long) /tmp/dosbox-x/src/hardware/mpu401.cpp:327:25
    #8 0x600f27cf8d31 in IO_WriteB(unsigned long, unsigned char) /tmp/dosbox-x/src/hardware/iohandler.cpp:500:3
    #9 0x600f287ad966 in CPU_Core_Normal_Run() /tmp/dosbox-x/src/cpu/./core_normal/prefix_none.h:1276:3
    #10 0x600f25097f88 in Normal_Loop() /tmp/dosbox-x/src/dosbox.cpp:374:23
    #11 0x600f25098ac3 in DOSBOX_RunMachine() /tmp/dosbox-x/src/dosbox.cpp:650:13
    #12 0x600f285b0933 in CALLBACK_RunRealInt(unsigned char) /tmp/dosbox-x/src/cpu/callback.cpp:227:2
    #13 0x600f2619ef89 in DOS_Shell::Execute(char*, char const*) /tmp/dosbox-x/src/shell/shell_misc.cpp:1496:3
    #14 0x600f2611c777 in DOS_Shell::DoCommand(char*) /tmp/dosbox-x/src/shell/shell_cmds.cpp:311:6
    #15 0x600f2606ee93 in DOS_Shell::ParseLine(char*) /tmp/dosbox-x/src/shell/shell.cpp:550:2
    #16 0x600f26090305 in DOS_Shell::Run() /tmp/dosbox-x/src/shell/shell.cpp:1071:4
    #17 0x600f2609c8e1 in SHELL_Run() /tmp/dosbox-x/src/shell/shell.cpp:1970:16
    #18 0x600f268c1b55 in VM_Boot_DOSBox_Kernel() /tmp/dosbox-x/src/gui/sdlmain.cpp:7413:9
    #19 0x600f2661d4f9 in BIOS::cb_bios_boot__func() /tmp/dosbox-x/src/ints/bios.cpp:11412:14
    #20 0x600f25098263 in Normal_Loop() /tmp/dosbox-x/src/dosbox.cpp:389:33
    #21 0x600f25098ac3 in DOSBOX_RunMachine() /tmp/dosbox-x/src/dosbox.cpp:650:13
    #22 0x600f268f891b in main /tmp/dosbox-x/src/gui/sdlmain.cpp:9340:13
    #23 0x70b244443ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #24 0x70b244443d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #25 0x600f24fbdc54 in _start (/tmp/dosbox-x/src/dosbox-x+0x38f7c54)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/usr/lib/libc.so.6+0xa6401) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) in memcpy
==91726==ABORTING

alsa-lib-git dosbox-x-3624d4b32a with running edge

tiwai commented 6 months ago

As mentioned in the above, it's a bug in the dosbox. It passes a wrong snd_seq_event_t flag bit, indicating as if it were a variable-length event such as sysex. This results in alsa-lib trying to take a bogus data pointer in the event data and segfaults. You need to initialize the event flag properly before submitting it.

tiwai commented 6 months ago

No bug in alsa-lib side, let's close