littlekernel / lk

LK embedded kernel
MIT License
3.12k stars 613 forks source link

[stm32f0cube][bugfix] Fix race which leads to 0-length SETUP transfers #253

Closed patrickshickel closed 4 years ago

patrickshickel commented 4 years ago

Fixes a race in the STM USB driver which can lead to the device seeing only 0-length SETUP transfers. The bug ultimately leads to a failed USB enumeration. The race condition is described in further detail below.

The current behavior of the IRQ handler for received OUT transfers is as such:

The important thing to note here is that even before RX_STAT is set to VALID in the last step, the HW can still receive and process a new SETUP transfer as long as RX_CTR is cleared (the spec has some detail about this under section "Control Transfers" on page 867: https://www.st.com/resource/en/reference_manual/dm00031936.pdf). The race will occur if we receive a SETUP transfer after clearing RX_CTR but before adjusting EP_RX_CNT back to max_packet. In this case, the IRQ handler will run for the newly received SETUP transfer, but the transfer will have no data associated with it (the driver ends up using the previous transfer data which was cached).

We can eliminate the race window by waiting to clear RX_CTR only after we've reset EP_RX_CNT. In this case the HW will ignore any new SETUP transfers until after the EP_RX_CNT is reset back to the desired value.

konkers commented 4 years ago

Looks good, Great Catch! Can we move bug description into the commit comment itself? That way it's discoverable through the git history.

patrickshickel commented 4 years ago

@konkers Done