raspberrypi / tinyusb

MIT License
68 stars 16 forks source link

Configuration descriptor parsing locks up if device configuration endpoint buffer is smaller than descriptor #6

Closed fieldofcows closed 2 years ago

fieldofcows commented 3 years ago

Set up

Describe the bug Tinyusb ends up in a tight loop freezing execution when attempting to enumerate a device that has a max control transfer data packet size that is less than the configuration description. This is due to the fact that in the control transfer, tinyusb assumes a SETUP->DATA1->ACK transfer, but if the control transfer requires more data than can fit in a single DATA packet, it could end up as SETUP->DATA1->DATA0->DATA1->DATAn-...->ACK.

This results in an incomplete descriptor in the transfer buffer which causes the parsing in parse_configuration_descriptor to end up in a loop that never exits.

To reproduce

  1. Setup a Raspberry Pi Pico development environment as described in https://datasheets.raspberrypi.org/pico/getting-started-with-pico.pdf
  2. Clone and build the pico-examples
  3. Connect the Pico to the Picoprobe and the Picoprobe to the development system
  4. Transfer and start debugging the USB HID host example
  5. Plug a USB1.1 mouse or keyboard into the target pico
  6. Break into execution using the debugger. You will see it is in a tight loop inside parse_configuration_descriptor

Log

USBH DEVICE ATTACH
Get 8 byte of Device Descriptor
Control Setup: 80 06 00 01 00 00 08 00 
on EP 00 with 8 bytes
on EP 80 with 8 bytes
Control data:
  0000:  12 01 10 01 00 00 00 08                          |........|
on EP 00 with 0 bytes
Port reset 
Set Address 
Control Setup: 00 05 01 00 00 00 00 00 
on EP 00 with 8 bytes
Control data:
NULL
on EP 80 with 0 bytes
Control Setup: 80 06 00 01 00 00 12 00 
on EP 00 with 8 bytes
on EP 80 with 8 bytes
Control data:
  0000:  12 01 10 01 00 00 00 08 00 00 00 00 00 00 00 00  |................|
  0010:  00 00                                            |..|
on EP 00 with 0 bytes
Get 9 bytes of Configuration Descriptor
Control Setup: 80 06 00 02 00 00 09 00 
on EP 00 with 8 bytes
on EP 80 with 8 bytes
Control data:
  0000:  09 02 22 00 01 01 00 A0 00                       |.."......|
on EP 00 with 0 bytes
Control Setup: 80 06 00 02 00 00 22 00 
on EP 00 with 8 bytes
on EP 80 with 8 bytes
Control data:
  0000:  09 02 22 00 01 01 00 A0 00 00 00 00 00 00 00 00  |..".............|         <---- Problem - Only first 8 bytes populated!
  0010:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  |................|
  0020:  00 00                                            |..|
on EP 00 with 0 bytes
ismell commented 3 years ago

So there is defiantly a hardware bug:

_hw_endpoint_xfer_sync: 0xa008:
 => High: Buffer Control: 0, Len: 0, Avail: 0, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 0
 => Low: * Buffer Control: 0xa008, Len: 8, Avail: 0, Stall: 0, Sel: 0, PID: 1, Last: 0, Full: 1 <- Looks good, we received 8 bytes
rx 8 bytes (buf_ctrl 0x0000a008)
  0000:  12 01 00 02 FF FF FF 08                          |........|
_hw_endpoint_xfer_continue: remaining_bytes: 10, ep->transfer_size: 8
_hw_endpoint_buffer_control_update32: original buffer_control: 0xa008, and_mask: 0, or_mask: 0x408
 => High: * Buffer Control: 0, Len: 0, Avail: 0, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 0
 => Low: Buffer Control: 0xa008, Len: 8, Avail: 0, Stall: 0, Sel: 0, PID: 1, Last: 0, Full: 1
_hw_endpoint_buffer_control_update32: Writing: 0x408
 <= Buffer Control: 0x408, Len: 8, Avail: 1, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 0 <-- Asking for the next 8 bytes / PID0.
buffer control (0x50100080) <- 0x408
buf_status 0x00000001
_hw_endpoint_xfer_sync: 0x80088000:
 => High: * Buffer Control: 0x8008, Len: 8, Avail: 0, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 1 <-- So this is expected. i.e., RP2040-E4.
 => Low: Buffer Control: 0x8000, Len: 0, Avail: 0, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 1 <-- This one is not expected. Why is FULL set here?
rx 8 bytes (buf_ctrl 0x00008008)
  0000:  5E 04 8E 02 10 01 01 02                          |^.......|
_hw_endpoint_xfer_continue: remaining_bytes: 2, ep->transfer_size: 2
Last buf (2 bytes left)
_hw_endpoint_buffer_control_update32: original buffer_control: 0x80088000, and_mask: 0, or_mask: 0x6402
 => High: Buffer Control: 0x8008, Len: 8, Avail: 0, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 1
 => Low: * Buffer Control: 0x8000, Len: 0, Avail: 0, Stall: 0, Sel: 0, PID: 0, Last: 0, Full: 1
_hw_endpoint_buffer_control_update32: Writing: 0x6402
 <= Buffer Control: 0x6402, Len: 2, Avail: 1, Stall: 0, Sel: 0, PID: 1, Last: 1, Full: 0 <- Asking for last 2 bytes / PID 1. 
buffer control (0x50100080) <- 0x6402
Stall REC

*** PANIC ***

Data Seq Error

I've verified the packets using a USB anaylzer and the device is sending the correct PIDs.

fieldofcows commented 3 years ago

Thanks for verifying @ismell. Unfortunately I haven't got access to a USB analyzer so could only go on the evidence of what I was seeing on the host. The workaround for this issue in my PR works for me. As far as I can tell, the workaround will result in the same packets being sent over the USB connection, but the host manages and handles each data packet as a separate transaction.