leighsmith / midisport-macos

M-Audio MIDISPORT USB 64-bit MIDI device driver for MacOS 10.14+
Other
124 stars 7 forks source link

Driver crash when sending large sysex message #26

Closed krevis closed 3 years ago

krevis commented 3 years ago

(Thanks very much for the work on getting this driver running again. The MIDISport 2x2 was probably the first MIDI device I got working on OS X, 20 years ago, and it's great to save it from the electronics graveyard.)

When I send a large sysex message through the MIDISport 2x2, via MIDISendSysex(), the driver is consistently crashing here:

Thread 4 Crashed:
0   libsystem_platform.dylib        0x00007fff20379236 _platform_memmove$VARIANT$Haswell + 566
1   com.leighsmith.midi.driver.midisport    0x0000000107a42bba MIDISPORT::PrepareOutput(InterfaceState*, std::__1::list<WriteQueueElem, std::__1::allocator<WriteQueueElem> >&, unsigned char*, unsigned long*, unsigned char*, unsigned long*) + 324
2   com.leighsmith.midi.driver.midisport    0x0000000107a445fa InterfaceState::DoWrite() + 98
3   com.leighsmith.midi.driver.midisport    0x0000000107a446f8 InterfaceState::WriteCallback(void*, int, void*) + 50
4   com.apple.framework.IOKit       0x00007fff22af26bb IODispatchCalloutFromCFMessage + 348
5   com.apple.CoreFoundation        0x00007fff2045aa88 __CFMachPortPerform + 288
6   com.apple.CoreFoundation        0x00007fff2042ea44 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 41
7   com.apple.CoreFoundation        0x00007fff2042e925 __CFRunLoopDoSource1 + 619
8   com.apple.CoreFoundation        0x00007fff2042cfaf __CFRunLoopRun + 2400
9   com.apple.CoreFoundation        0x00007fff2042bf8c CFRunLoopRunSpecific + 563
10  com.apple.audio.midi.CoreMIDI   0x00007fff34d7246d DriverIOThread::Run() + 49
11  com.apple.audio.midi.CoreMIDI   0x00007fff34d724e2 XThread::RunHelper(void*) + 10
12  com.apple.audio.midi.CoreMIDI   0x00007fff34d7365b CAPThread::Entry(CAPThread*) + 77
13  libsystem_pthread.dylib         0x00007fff203368fc _pthread_start + 224
14  libsystem_pthread.dylib         0x00007fff20332443 thread_start + 15

Full crash log: MIDIServer_2021-10-16-213858_lite.crash.zip

Steps:

  1. Running on Big Sur 11.6, Install the driver via MIDISPORTDriver-v1.2.0.pkg. Plug in the MIDISport 2x2, etc.
  2. Download and run SysEx Librarian (current version: 1.4.1). I'm the author.
  3. Get a large sysex file like this one: 64KB.syx.zip.
  4. Unzip it and drag it into the SysEx Librarian window.
  5. In the Destination pop-up menu, choose one of the MIDISport’s ports
  6. Select the sysex file, then press the Play button. Let it finish playing or cancel after a little while, doesn't matter.

Expected: The whole 64 KB sysex message is transmitted, and nothing bad happens.

Actual: The MIDISport's Out LED goes on for less than a second, then off. There's a MIDIServer crash in Console. (SysEx Librarian also acts kind of erratically afterwards, triggered by the MIDIServer crash and restart, but that's my problem to solve.)

Something about this long sysex message seems to be triggering the crash. I didn’t see it with small sysex messages. Haven't worked out exactly how long is too long, yet.

krevis commented 3 years ago

I'll get the code and start investigating. If this is a known bug, or if you have any pointers on where to start looking, I'd love to know more.

krevis commented 3 years ago

The crash happens here:

            case 0x0: case 0x1: case 0x2: case 0x3:
            case 0x4: case 0x5: case 0x6: case 0x7:
                // DebugPrintf("sysex databyte %02X", c);
                // data byte, presumably a sysex continuation
                *dest[cableEndpoint]++ = c;
                numToBeSent = srcend - src;
                // sysex ends with 2 preceding data bytes or sysex continues, such that the 
                // sysex end message begins the packet as the cmd.
                outPacketLen = (numToBeSent >= 3) ? 2 : numToBeSent - 1;    

                // DebugPrintf("outPacketLen = %d, numToBeSent = %d", outPacketLen, numToBeSent);
                memcpy(dest[cableEndpoint], src, outPacketLen); // <------ CRASH HERE
                memset(dest[cableEndpoint] + outPacketLen, 0, 2 - outPacketLen);
                dest[cableEndpoint][2] = cableNibble | (outPacketLen + 1); // mark length and cable
                dest[cableEndpoint] += (MIDIPACKETLEN - 1);   // we advance by one packet length (4 bytes)
                src += outPacketLen;
                break;

This is the last byte of data to write (src == srcend), so numToBeSent is 0, outPacketLen is -1, and we try to memcpy too much.