From 3ef97296e7b512675577acf7d716e10d06615189 Mon Sep 17 00:00:00 2001 From: Udipto Goswami Date: Tue, 6 Apr 2021 14:46:10 +0530 Subject: [PATCH] usb: dwc3: gadget: Handle race between remove_request & ep_dequeue Consider a case where remove_request from ep_disable & ep_dequeue from function driver got called at the same time. In this case due to release the spinlock at the time of giveback we would end up mixing the execution of both the processes and would interfere in the list operations. Consider the following example: remove_request | | V STARTED_LIST | | V gadget_giveback (release_lock) ep_dequeue (acquired_lock) | | V STARTED_LIST | | V move_cancelled_list | | V gadget_giveback (release_lock) (acquired_lock) rest of the started list empty CANCELLED_LIST | | V gadget_giveback (release_lock) (acquired_lock) gadget_giveback [KERNEL_PANIC] This is beause the cleanup_cancelled_request uses list_for_each_entry_safe which caches the next node of the list in a temp variable. Now when the lock is release the remove_request process will free up that request. But the pointer was not updated for cancelled_request. So the next time is acquired the lock it will access a stray pointer which will lead to kernel panic. Fix this by ensuring that when remove_request is called, ep_dequeue does not execute cleanup_cancelled_request. The stop active transfer from remove_request will clear the transfer started flag therefore make sure ep_dequeue cleanup_cancelled_request executed only when the flag is set along the transfer pending bit. Additionally adding check in stop_active_transfer,if ep cmd failure with -ETIMEDOUT is seen then don't modify the flags at all. Change-Id: If17c2901e572d071808c2e71ff2d9476ce56c30d Signed-off-by: Udipto Goswami --- drivers/usb/dwc3/gadget.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index d7cc25635afc..9d4a79ad4306 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1787,7 +1787,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, /* If ep isn't started, then there's no end transfer * pending */ - if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) + if ((dep->flags & DWC3_EP_TRANSFER_STARTED) && + !(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) dwc3_gadget_ep_cleanup_cancelled_requests(dep); goto out; @@ -3461,8 +3462,11 @@ int dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool interrupt) ret = dwc3_send_gadget_ep_cmd(dep, cmd, ¶ms); WARN_ON_ONCE(ret); dep->resource_index = 0; - - dbg_log_string("%s(%d): endxfer ret:%d", dep->name, dep->number, ret); + if (ret == -ETIMEDOUT) { + dbg_log_string("%s(%d): endxfer ret:%d", + dep->name, dep->number, ret); + return ret; + } if (!interrupt) dep->flags &= ~DWC3_EP_TRANSFER_STARTED;