Vendor Device race condition

[kasjer]

Operating System

Windows 10

Board

pca10095

Firmware

custom firmware running mynewt OS

What happened ?

Problem was detected on IN transfers over vendor interface on NRF5340 (but it's not MCU specific). It may be that other driver has similar problem. Here is sequence that fails.

• Vendor interface IN packet is sent to host.
• vendord_xfer_cb() is called
• it calls maybe_transmit()
• endpoint is not busy (yet) TU_VERIFY( !usbd_edpt_busy(TUD_OPT_RHPORT, p_itf->ep_in) ); goes through
• there is more data in FIFO already; endpoint data is prepared
• next transfer is started: TU_ASSERT( usbd_edpt_xfer(TUD_OPT_RHPORT, p_itf->ep_in, p_itf->epin_buf, count) );
• assert fails TU_ASSERT(_usbd_dev.ep_status[epnum][dir].busy == 0);

In time between checking if endpoint is busy and setting busy state additional transfer was started from other thread or interrupt. From time to time TU_ASSERT(_usbd_dev.ep_status[epnum][dir].busy == 0); goes through in both threads and both threads try to start next transfer.

To workaround this problem I call usbd_defer_func() to run function that would start next transfer in USBD task context to eliminate race condition. This way it works as it would in non-OS scenario where race is eliminated by single thread. However usbd_defer_func is declared in header that indicates that it's privet and I'm not sure that I should use it application.

Any advice how to solve this properly? Is starting of transfer from interrupt allowed? Is scheduling function to run in USBD task from other thread/isr supported?

I did not check but it may be that problem is not limited to IN transfer of vendor interface only.

How to reproduce ?

There is no easy way to reproduce as there is no public example of vendor interface usage.

Debug Log as txt file

No response

Screenshots

No response

[hathach]

The issue seems to be the stack's vendor driver, currently it doesn't guard against race condition as cdc. I haven't updated the vendor driver once introducing the usbd_edpt_claim() for race protection. I actually have an plan to abstract these to stream/buffered endpoints API for driver and haven't got time to do so.

https://github.com/hathach/tinyusb/blob/master/src/class/vendor/vendor_device.c#L109 https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.c#L83

Originally posted by @hathach in https://github.com/hathach/tinyusb/issues/900#issuecomment-870261839

Yeah, the vendor driver isn't updated to guard against race condition (would be something similar to cdc). I was planning to update + refactor bulk endpoint usage (midi/cdc/vendor) to new type called endpoint stream but haven't got time to do so.