[Bug] Bluepad crashing when connecting Switch Pro controller devices

[juan518munoz]

Found on Pico W platform:

Bluepad32 crashes when a connecting controller identified as a switch calls process_reply_spi_flash_read function. I suspect it's the following lines:

    switch_instance_t* ins = get_switch_instance(d);
    switch (ins->state) {

Reverting to an old version of code where mem_len was used:

    switch (mem_len) {

Solves the issue.

I suspect ins or ins->state might be NULL and causes a segfault.

Feel free to request logs or further info if needed.

[ricardoquesada]

very interesting... which controller are you using ?

[ricardoquesada]

also, please attach the logs from the console... it should never return NULL... wondering if the memory got corrupted.

and does it get reproduced from the example/pico project ?

[juan518munoz]

Tested on commit with first pico W example, experienced the issue with most recent changes too.

Issue is reproducible in a Joycon L, Joycon R (both firmware 4.25) and a generic Switch Pro Controller (no specified firmware in logs).

For logging I the code is laid out like this:

// Reply to SUBCMD_SPI_FLASH_READ
static void process_reply_spi_flash_read(struct uni_hid_device_s* d, const struct switch_report_21_s* r, int len) {
    logi("Entering: process_reply_spi_flash_read\n");
    int mem_len = r->data[4];
    logi("mem_len: %d\n", mem_len);
    uint32_t addr = r->data[0] | r->data[1] << 8 | r->data[2] << 16 | r->data[3] << 24;
    logi("addr: 0x%04x\n", addr);

    switch_instance_t* ins = NULL;
    logi("about to get switch instance\n");
    ins = get_switch_instance(d);

    if(!ins) {
        logi("ins is NULL\n");
        return;
    }

    logi("ins state: %d\n", ins->state);
    switch (ins->state) {
        case STATE_READ_FACTORY_STICK_CALIBRATION:
            process_reply_read_spi_factory_stick_calibration(d, r->data, mem_len + 5);
            break;
        case STATE_READ_USER_STICK_CALIBRATION:
            process_reply_read_spi_user_stick_calibration(d, r->data, mem_len + 5);
            break;
        case STATE_READ_FACTORY_IMU_CALIBRATION:
            process_reply_read_spi_factory_imu_calibration(d, r->data, mem_len + 5);
            break;
        case STATE_DUMP_FLASH:
            process_reply_read_spi_dump(d, r->data, mem_len + 5);
            break;
        default:
            loge("Switch: unexpected spi_read size reply %d at 0x%04x\n", mem_len, addr);
            printf_hexdump((const uint8_t*)r, len);
    };
    logi("Exiting: process_reply_spi_flash_read\n\n");
}

This is the resulting output:

2CAP_EVENT_CHANNEL_OPENED (channel=0x0042)
PSM: 0x0013, local CID=0x0042, remote CID=0x0041, handle=0x000b, incoming=0, local5
HID Interrupt opened, cid 0x42
Device 80:D2:E5:2A:D6:55 is connected
my_platform: device connected: 0x2000396c
uni_bt_process_fsm, bd addr:80:D2:E5:2A:D6:55,  state: 12, incoming:0
uni_bt_process_fsm: Device is ready
Switch: IMU report enabled
Switch: Firmware version: 4.25. Controller type=1
Entering: process_reply_spi_flash_read
mem_len: 18
addr: 0x603d
about to get switch instance
ins state: 3
Switch: Stick calibration info: x=587,1827,3231, y=1080,2225,3384, rx=0,4095,8190,0
Exiting: process_reply_spi_flash_read

Entering: process_reply_spi_flash_read
mem_len: 22
addr: 0x8010
about to get switch instance
ins state: 4
Exiting: process_reply_spi_flash_read

Entering: process_reply_spi_flash_read
mem_len: 24
addr: 0x6020

The last logi("about to get switch instance\n"); should be printed, maybe it crashes before it gets flushed to stdio.

[juan518munoz]

Confirmed it's not getting flushed on time due to the pico crashing, adding sleep_ms(10000) before logi("about to get switch instance\n"); gets the pico up to here:

...
Entering: process_reply_spi_flash_read
mem_len: 22
addr: 0x8010
about to get switch instance
ins state: 4
Exiting: process_reply_spi_flash_read

Entering: process_reply_spi_flash_read
mem_len: 24
addr: 0x6020
about to get switch instance
ins state: 

[ricardoquesada]

I can repro on Pico W.
Cannot repro on Linux.
Cannot repro on ESP32.

weird

[ricardoquesada]

please try again with latest develop branch .fixed.

[juan518munoz]

Looks ok now