← index #6953PR #3779
Related · medium · value 0.370
QUERY · ISSUE

stm32 HID class USB crash explained

openby doc-hexopened 2021-02-23updated 2024-09-13

Problem: If you call pyb.USB_HID().recv(...) before the USB device is enumerated by the host, a null pointer is dereferenced and hard fault results immediately.

This happens because the HID interface object is not initialized: usb_device.usbd_cdc_msc_hid_state.hid and in particular usb_device.usbd_cdc_msc_hid_state.hid.usbd are zeros.

The init should happen:

  • when the device is enumerated (SetConfig chapter 9 code) in USBD_SetClassConfig
  • calls USBD_CDC_MSC_HID_Init which calls usbd_cdc_state_init
  • I could not find any other paths that might initialize the usbd field in particular; the other fields are also wrong, such as defaulting to dev_speed == USBD_SPEED_HIGH even on my board which can't do high speed, because USBD_SPEED_HIGH is zero.

The null de-ref happens inside USBD_HID_ReceivePacket(&hid->base...) when called by usbd_hid_rx() and it only gets that far (thinking that zero bytes have been received already) because it looks at the un-initialized usbd_hid_itf_t *hid and sees zero for hid->report_in_len rather than USBD_HID_REPORT_INVALID which would be more appropriate in this state.

My workaround, on ports/stm32/usbd_hid_interface.c:

--- a/ports/stm32/usbd_hid_interface.c
+++ b/ports/stm32/usbd_hid_interface.c
@@ -49,7 +49,7 @@ int8_t usbd_hid_receive(usbd_hid_state_t *hid_in, size_t len) {
 int usbd_hid_rx(usbd_hid_itf_t *hid, size_t len, uint8_t *buf, uint32_t timeout_ms) {
     // Wait for an incoming report
     uint32_t t0 = mp_hal_ticks_ms();
-    while (hid->report_in_len == USBD_HID_REPORT_INVALID) {
+    while (hid->report_in_len == USBD_HID_REPORT_INVALID || !hid->base.usbd) {
         if (mp_hal_ticks_ms() - t0 >= timeout_ms || query_irq() == IRQ_STATE_DISABLED) {
             return -MP_ETIMEDOUT;
         }
@@ -62,6 +62,7 @@ int usbd_hid_rx(usbd_hid_itf_t *hid, size_t len, uint8_t *buf, uint32_t timeout_
 
     // Schedule to receive next incoming report
     hid->report_in_len = USBD_HID_REPORT_INVALID;
+    assert(hid->base.usbd);
     USBD_HID_ReceivePacket(&hid->base, &hid->report_in_buf[0]);

I can't explain why this bug isn't seen more often, but I think what was covering it for me was that pyb_usb_dev_init() gets called after the end of boot.py and so was defaulting to VCP+HID+MSC but I've recently changed things so that USB doesn't get enabled until sometime much later in main.py (ie. I called pyb.usb_mode(None) in boot.py) and that initialized the composite USB device as "VCP+HID". I suspect something in the code path for the MSC setup initializes the HID interface state before enumeration, but I can't confirm that.

CANDIDATE · PULL REQUEST

usbd_cdc_msc_hid.c: bugfix: be honest about data not written to EP

closedby doc-hexopened 2018-05-11updated 2018-05-14

If the host is slow reading data, we can detect that case with this change.

Keyboard

j / / n
next pair
k / / p
previous pair
1 / / h
show query pane
2 / / l
show candidate pane
c
copy suggested comment
r
toggle reasoning
g i
go to index
?
show this help
esc
close overlays

press ? or esc to close

copied