Skip to content

[nrf fromtree] usb: host: fix control transfer ownership in usbh_req_… - #4607

Open
andu-nordic wants to merge 1 commit into
nrfconnect:ncs-v3.4-branchfrom
andu-nordic:pick/v3.4/usb-fixup-host
Open

andu-nordic wants to merge 1 commit into
nrfconnect:ncs-v3.4-branchfrom
andu-nordic:pick/v3.4/usb-fixup-host

Conversation

@andu-nordic

Copy link
Copy Markdown
Contributor

…setup

usbh_req_setup() waits on a single global semaphore with no mutual exclusion, so two threads can be inside it at once: enumeration runs on the bus thread while the shell and class drivers call in from their own. The first completion wakes an arbitrary waiter, which then reads xfer->err on a transfer that has not finished. Serialize the request with a mutex so the semaphore has one owner at a time.

Timing out also released the transfer while the controller still owned it. uhc_ep_dequeue() only requests cancellation: the transfer stays with the driver until it is returned through the completion callback, and the driver may still write into its buffer. Callers such as usbh_req_desc_dev() free that buffer as soon as usbh_req_setup() returns, so a cancelled transfer could land in a recycled net_buf. Wait for the completion callback before freeing, and drop the callback's -ECONNRESET path now that the caller always owns the transfer.

Signed-off-by: Ofir Shemesh ofirshemesh777@gmail.com

(cherry picked from commit 6862873)

…setup

usbh_req_setup() waits on a single global semaphore with no mutual
exclusion, so two threads can be inside it at once: enumeration runs
on the bus thread while the shell and class drivers call in from
their own. The first completion wakes an arbitrary waiter, which then
reads xfer->err on a transfer that has not finished. Serialize the
request with a mutex so the semaphore has one owner at a time.

Timing out also released the transfer while the controller still
owned it. uhc_ep_dequeue() only requests cancellation: the transfer
stays with the driver until it is returned through the completion
callback, and the driver may still write into its buffer. Callers
such as usbh_req_desc_dev() free that buffer as soon as
usbh_req_setup() returns, so a cancelled transfer could land in a
recycled net_buf. Wait for the completion callback before freeing,
and drop the callback's -ECONNRESET path now that the caller always
owns the transfer.

Signed-off-by: Ofir Shemesh <ofirshemesh777@gmail.com>

(cherry picked from commit 6862873ee28efc6bbac5553de365cd0d92d1b55f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants