[USB Serial JTAG] calling select() & poll() crashes (IDFGH-8509)

[chipweinberger]

v4.4.2
ESP32-S3

What is the expected behavior?

poll() and select() should be implemented for USB Serial JTAG. Is there a reason they are not?

relevant code: https://github.com/espressif/esp-idf/blob/master/components/vfs/vfs_usb_serial_jtag.c

I need poll() or select() to determine if bytes are waiting on stdin.

Use Case: If the user enters any input during boot, USB Host is disabled and instead the user enters a Usb Serial Console.

What is the actual behavior?

The code crashes. Null ptr deref trying to execute the non-existent function.

(This was very annoying to debug due to the virtual pointers! At the least we should log + assert😀)

Steps to reproduce.

1. Enable Usb Serial JTAG as the Primary Console in Menuconfig
2. call poll() or select() on stdin.

Code:

int stdin_has_data()
{
    struct pollfd fds[2];
    fds[0].fd = 0; // 0 is stdin
    fds[0].events = POLLIN;
    int timeout_msecs = 0;
    int ret = poll(fds, 1, timeout_msecs);
    if (ret < 0) {
        ESP_LOGE(TAG, "poll() failed");
        return false;
    }

    return (fds[0].revents & POLLIN) != 0;
}

Debug Logs.

    //printf("write %p\n", get_vfs_for_index(primary_vfs_index)->vfs.write);
    //printf("open %p\n", get_vfs_for_index(primary_vfs_index)->vfs.open);
    //printf("fstat %p\n", get_vfs_for_index(primary_vfs_index)->vfs.fstat);
    //printf("close %p\n", get_vfs_for_index(primary_vfs_index)->vfs.close);
    //printf("read %p\n", get_vfs_for_index(primary_vfs_index)->vfs.read);
    //printf("fcntl %p\n", get_vfs_for_index(primary_vfs_index)->vfs.fcntl);
    //printf("fsync %p\n", get_vfs_for_index(primary_vfs_index)->vfs.fsync);
    //printf("access %p\n", get_vfs_for_index(primary_vfs_index)->vfs.access);
    //printf("start_select %p\n", get_vfs_for_index(primary_vfs_index)->vfs.start_select);
    //printf("end_select %p\n", get_vfs_for_index(primary_vfs_index)->vfs.end_select);
    //printf("tcsetattr %p\n", get_vfs_for_index(primary_vfs_index)->vfs.tcsetattr);
    //printf("tcgetattr %p\n", get_vfs_for_index(primary_vfs_index)->vfs.tcgetattr);
    //printf("tcdrain %p\n", get_vfs_for_index(primary_vfs_index)->vfs.tcdrain);
    //printf("tcflush %p\n", get_vfs_for_index(primary_vfs_index)->vfs.tcflush);

write 0x4200884c
0x4200884c: usb_serial_jtag_write at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:139

open 0x42008410
0x42008410: usb_serial_jtag_open at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:103

fstat 0x420086c4
0x420086c4: usb_serial_jtag_fstat at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:233

close 0x420084e8
0x420084e8: usb_serial_jtag_close at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:240

read 0x42008924
0x42008924: usb_serial_jtag_read at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:188

fcntl 0x420085d8
0x420085d8: usb_serial_jtag_fcntl at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:245

fsync 0x420086fc
0x420086fc: usb_serial_jtag_fsync at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:262

access 0x0
start_select 0x0
end_select 0x0
tcsetattr 0x420087c4
0x420087c4: usb_serial_jtag_tcsetattr at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:280

tcgetattr 0x4200863c
0x4200863c: usb_serial_jtag_tcgetattr at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:312

tcdrain 0x42008798
0x42008798: usb_serial_jtag_tcdrain at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:337

tcflush 0x420085a8
0x420085a8: usb_serial_jtag_tcflush at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs_usb_serial_jtag.c:343


Guru Meditation Error: Core  0 panic'ed (InstrFetchProhibited). Exception was unhandled.

Core  0 register dump:
PC      : 0x00000000  PS      : 0x00060530  A0      : 0x82007d27  A1      : 0x3fcf5c10  
A2      : 0x00000001  A3      : 0x00000000  A4      : 0x00000000  A5      : 0x00000000  
A6      : 0x00000000  A7      : 0x00000000  A8      : 0x8200837f  A9      : 0x3fcf5be0  
A10     : 0x00000001  A11     : 0x00000000  A12     : 0x00000000  A13     : 0x00000000  
A14     : 0x3c174a01  A15     : 0x3fce1508  SAR     : 0x00000018  EXCCAUSE: 0x00000014  
EXCVADDR: 0x00000000  LBEG    : 0x40056f08  LEND    : 0x40056f12  LCOUNT  : 0x00000000  


Backtrace: 0xfffffffd:0x3fcf5c10 0x42007d24:0x3fcf5c60 0x42106852:0x3fcf5cd0 0x4200d6d9:0x3fcf5d20 0x4200d768:0x3fcf5d60 0x4200ed3d:0x3fcf5d90 0x4216de54:0x3fcf5e60 0x40389eaa:0x3fcf5e90
0x42007d24: esp_vfs_select at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/vfs/vfs.c:1013 (discriminator 15)

0x42106852: poll at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/esp-idf/components/newlib/poll.c:71 (discriminator 4)

0x4200d6d9: stdin_has_data() at /Volumes/User/MBP-Google-Drive/jamcorder/firmware/jamcorder-firmware/jamcorder_app/main/pd_main.cpp:743

More Information.

No response

[chipweinberger]

We crash in vfs_console.c because get_vfs_for_index(primary_vfs_index)->vfs.start_select is NULL. We should check if it is null and return an error...

vfs_console.c

static esp_err_t console_start_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
        esp_vfs_select_sem_t select_sem, void **end_select_args)
{
    // nullptr deref here
    return get_vfs_for_index(primary_vfs_index)->vfs.start_select(nfds, readfds, writefds, exceptfds, select_sem, end_select_args);
}

Note: the null check we do in vfs.c is insufficent, because that points to console_start_select, which is valid. But usb_serial_jtag_start_select is still NULL.

vfs.c

        // vfs->vfs.start_select is valid, so this check doesn't work as it should 
        if (vfs && vfs->vfs.start_select && item->isset) {
            // call start_select for all non-socket VFSs with has at least one FD set in readfds, writefds, or errorfds
            // note: it can point to socket VFS but item->isset will be false for that
            ESP_LOGD(TAG, "calling start_select for VFS ID %d with the following local FDs", i);
            esp_vfs_log_fd_set("readfds1", &item->readfds);
            esp_vfs_log_fd_set("writefds", &item->writefds);
            esp_vfs_log_fd_set("errorfds", &item->errorfds);
            esp_err_t err = vfs->vfs.start_select(nfds, &item->readfds, &item->writefds, &item->errorfds, sel_sem,
                    driver_args + i);

[adokitkat]

Hello. Thank you for the report. I've fixed the crash issue (backports yet to be merged), however unfortunately I don't have much knowledge about select() and poll() so I created a new issue in our internal Jira about their implementation and I'll keep this issue open.

[SoucheSouche]

Hi @chipweinberger,
I implemented the select() for USB serial JTAG e week ago. The review is ongoing internally.