michaelh/zephyr
1
0
Fork 0
Code Releases Activity

drivers: usb: udc_nrf: fix race condition in nRF USBD

Browse source 
Periodic enqeueu of buffers can cause a attempt to start
a new transfer to host even though an IN endpoint is already busy.
Use busy state flags to explicitly mark an endpoint busy.

Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>
This commit is contained in:
Johann Fischer 2022-12-29 01:47:08 +01:00 • committed by Carles Cufí
commit a033784c7e
1 changed files with 20 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

26
drivers/usb/udc/udc_nrf.c
View file

@ -88,7 +88,7 @@ static void udc_event_xfer_in_next(const struct device *dev, const uint8_t ep)
{ {
struct net_buf *buf; struct net_buf *buf;
if (nrfx_usbd_ep_is_busy(ep)) { if (udc_ep_is_busy(dev, ep)) {
return; return;
} }
@ -108,6 +108,8 @@ static void udc_event_xfer_in_next(const struct device *dev, const uint8_t ep)
/* REVISE: remove from endpoint queue? ASSERT? */ /* REVISE: remove from endpoint queue? ASSERT? */
udc_submit_event(dev, UDC_EVT_EP_REQUEST, udc_submit_event(dev, UDC_EVT_EP_REQUEST,
-ECONNREFUSED, buf); -ECONNREFUSED, buf);
} else {
udc_ep_set_busy(dev, ep, true);
} }
} }
} }
@ -165,6 +167,7 @@ static void udc_event_xfer_in(const struct device *dev,
return; return;
} }
udc_ep_set_busy(dev, ep, false);
if (ep == USB_CONTROL_EP_IN) { if (ep == USB_CONTROL_EP_IN) {
return udc_event_xfer_ctrl_in(dev, buf); return udc_event_xfer_ctrl_in(dev, buf);
} }
@ -181,6 +184,7 @@ static void udc_event_xfer_in(const struct device *dev,
return; return;
} }
udc_ep_set_busy(dev, ep, false);
udc_submit_event(dev, UDC_EVT_EP_REQUEST, udc_submit_event(dev, UDC_EVT_EP_REQUEST,
-ECONNABORTED, buf); -ECONNABORTED, buf);
break; break;
@ -213,6 +217,10 @@ static void udc_event_xfer_out_next(const struct device *dev, const uint8_t ep)
{ {
struct net_buf *buf; struct net_buf *buf;
if (udc_ep_is_busy(dev, ep)) {
return;
}
buf = udc_buf_peek(dev, ep, true); buf = udc_buf_peek(dev, ep, true);
if (buf != NULL) { if (buf != NULL) {
nrfx_usbd_transfer_t xfer = { nrfx_usbd_transfer_t xfer = {
@ -228,6 +236,8 @@ static void udc_event_xfer_out_next(const struct device *dev, const uint8_t ep)
/* REVISE: remove from endpoint queue? ASSERT? */ /* REVISE: remove from endpoint queue? ASSERT? */
udc_submit_event(dev, UDC_EVT_EP_REQUEST, udc_submit_event(dev, UDC_EVT_EP_REQUEST,
-ECONNREFUSED, buf); -ECONNREFUSED, buf);
} else {
udc_ep_set_busy(dev, ep, true);
} }
} else { } else {
LOG_DBG("ep 0x%02x waiting, queue is empty", ep); LOG_DBG("ep 0x%02x waiting, queue is empty", ep);
@ -244,7 +254,10 @@ static void udc_event_xfer_out(const struct device *dev,
switch (event->data.eptransfer.status) { switch (event->data.eptransfer.status) {
case NRFX_USBD_EP_WAITING: case NRFX_USBD_EP_WAITING:
udc_event_xfer_out_next(dev, ep); /*
* There is nothing to do here, new transfer
* will be tried in both cases later.
*/
break; break;
case NRFX_USBD_EP_OK: case NRFX_USBD_EP_OK:
@ -260,6 +273,7 @@ static void udc_event_xfer_out(const struct device *dev,
} }
net_buf_add(buf, len); net_buf_add(buf, len);
udc_ep_set_busy(dev, ep, false);
if (ep == USB_CONTROL_EP_OUT) { if (ep == USB_CONTROL_EP_OUT) {
udc_event_xfer_ctrl_out(dev, buf); udc_event_xfer_ctrl_out(dev, buf);
} else { } else {
@ -344,9 +358,9 @@ static void udc_nrf_thread(const struct device *dev)
ep = evt.hal_evt.data.eptransfer.ep; ep = evt.hal_evt.data.eptransfer.ep;
switch (evt.hal_evt.type) { switch (evt.hal_evt.type) {
case NRFX_USBD_EVT_EPTRANSFER: case NRFX_USBD_EVT_EPTRANSFER:
start_xfer = true;
if (USB_EP_DIR_IS_IN(ep)) { if (USB_EP_DIR_IS_IN(ep)) {
udc_event_xfer_in(dev, &evt.hal_evt); udc_event_xfer_in(dev, &evt.hal_evt);
start_xfer = true;
} else { } else {
udc_event_xfer_out(dev, &evt.hal_evt); udc_event_xfer_out(dev, &evt.hal_evt);
} }
@ -368,11 +382,9 @@ static void udc_nrf_thread(const struct device *dev)
} }
if (start_xfer) { if (start_xfer) {
struct udc_ep_config *cfg = udc_get_ep_cfg(dev, ep);
if (USB_EP_DIR_IS_IN(ep)) { if (USB_EP_DIR_IS_IN(ep)) {
udc_event_xfer_in_next(dev, ep); udc_event_xfer_in_next(dev, ep);
} else if (cfg->stat.pending) { } else {
udc_event_xfer_out_next(dev, ep); udc_event_xfer_out_next(dev, ep);
} }
} }
@ -517,6 +529,8 @@ static int udc_nrf_ep_dequeue(const struct device *dev,
} }
udc_ep_set_busy(dev, cfg->addr, false);
return 0; return 0;
} }