lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 12 Oct 2016 19:37:31 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     balbi@...nel.org
Cc:     gregkh@...uxfoundation.org, broonie@...nel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        baolin.wang@...aro.org
Subject: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

When we change the USB function with configfs frequently, sometimes it will
hang the system to crash. The reason is the gadget driver can not hanle the
end transfer complete event after free the gadget irq (since the xHCI will
share the same interrupt number with gadget, thus when free the gadget irq,
it will not shutdown this gadget irq line), which will trigger the interrupt
all the time.

Thus we should check if we need wait for the end transfer command completion
before free gadget irq.

Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
---
Changes since v1:
 - Simply the operation of cleaning the dep flags.
---
 drivers/usb/dwc3/core.h   |    3 ++
 drivers/usb/dwc3/gadget.c |   73 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 9128725..f01d8fd 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -537,6 +537,7 @@ struct dwc3_ep {
 #define DWC3_EP_BUSY		(1 << 4)
 #define DWC3_EP_PENDING_REQUEST	(1 << 5)
 #define DWC3_EP_MISSED_ISOC	(1 << 6)
+#define DWC3_EP_CMDCMPLT_BUSY	(1 << 7)
 
 	/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN		(1 << 31)
@@ -746,6 +747,7 @@ struct dwc3_scratchpad_array {
  * @ep0_bounce_addr: dma address of ep0_bounce
  * @scratch_addr: dma address of scratchbuf
  * @ep0_in_setup: One control transfer is completed and enter setup phase
+ * @cmd_complete: endpoint command completion
  * @lock: for synchronizing
  * @dev: pointer to our struct device
  * @xhci: pointer to our xHCI child
@@ -845,6 +847,7 @@ struct dwc3 {
 	dma_addr_t		scratch_addr;
 	struct dwc3_request	ep0_usb_req;
 	struct completion	ep0_in_setup;
+	struct completion	cmd_complete;
 
 	/* device lock */
 	spinlock_t		lock;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index fef023a..32e3d4d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -573,6 +573,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
 		dep->comp_desc = comp_desc;
 		dep->type = usb_endpoint_type(desc);
 		dep->flags |= DWC3_EP_ENABLED;
+		dep->flags &= ~DWC3_EP_CMDCMPLT_BUSY;
 
 		reg = dwc3_readl(dwc->regs, DWC3_DALEPENA);
 		reg |= DWC3_DALEPENA_EP(dep->number);
@@ -650,7 +651,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 	dep->endpoint.desc = NULL;
 	dep->comp_desc = NULL;
 	dep->type = 0;
-	dep->flags = 0;
+	dep->flags &= DWC3_EP_CMDCMPLT_BUSY;
 
 	return 0;
 }
@@ -1732,6 +1733,54 @@ static void __dwc3_gadget_stop(struct dwc3 *dwc)
 	__dwc3_gadget_ep_disable(dwc->eps[1]);
 }
 
+static void dwc3_wait_command_complete(struct dwc3 *dwc)
+{
+	u32 epnum, epstart = 2;
+	int ret, wait_cmd_complete = 0;
+	unsigned long flags;
+
+check_next:
+	spin_lock_irqsave(&dwc->lock, flags);
+	/*
+	 * If the gadget has been in suspend state, then don't
+	 * need to wait for the end transfer complete event.
+	 */
+	if (pm_runtime_suspended(dwc->dev)) {
+		spin_unlock_irqrestore(&dwc->lock, flags);
+		return;
+	}
+
+	for (epnum = epstart; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
+		struct dwc3_ep *dep;
+
+		dep = dwc->eps[epnum];
+		if (!dep)
+			continue;
+
+		if (dep->flags & DWC3_EP_CMDCMPLT_BUSY) {
+			reinit_completion(&dwc->cmd_complete);
+			epstart = epnum + 1;
+			wait_cmd_complete = 1;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	/*
+	 * Wait for 500ms to complete the end transfer command before free irq.
+	 */
+	if (wait_cmd_complete) {
+		wait_cmd_complete = 0;
+		ret = wait_for_completion_timeout(&dwc->cmd_complete,
+						  msecs_to_jiffies(500));
+		if (ret == 0)
+			dev_warn(dwc->dev,
+				 "timeout to wait for command complete.\n");
+
+		goto check_next;
+	}
+}
+
 static int dwc3_gadget_stop(struct usb_gadget *g)
 {
 	struct dwc3		*dwc = gadget_to_dwc(g);
@@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
 	dwc->gadget_driver	= NULL;
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
+	/*
+	 * Since the xHCI will share the same interrupt with gadget, thus when
+	 * free the gadget irq, it will not shutdown this gadget irq line. Then
+	 * the gadget driver can not handle the end transfer command complete
+	 * event after free the gadget irq, which will hang the system to crash.
+	 *
+	 * So we should wait for the end transfer command complete event before
+	 * free it to avoid this situation.
+	 */
+	dwc3_wait_command_complete(dwc);
+
 	free_irq(dwc->irq_gadget, dwc->ev_buf);
 
 	return 0;
@@ -2108,7 +2168,8 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
 
 	dep = dwc->eps[epnum];
 
-	if (!(dep->flags & DWC3_EP_ENABLED))
+	if (!(dep->flags & DWC3_EP_ENABLED) &&
+	    !(dep->flags & DWC3_EP_CMDCMPLT_BUSY))
 		return;
 
 	if (epnum == 0 || epnum == 1) {
@@ -2180,6 +2241,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
 		dwc3_trace(trace_dwc3_gadget, "%s FIFO Overrun", dep->name);
 		break;
 	case DWC3_DEPEVT_EPCMDCMPLT:
+		if (dep->flags & DWC3_EP_CMDCMPLT_BUSY) {
+			dep->flags &= ~DWC3_EP_CMDCMPLT_BUSY;
+			complete(&dwc->cmd_complete);
+		}
+
 		dwc3_trace(trace_dwc3_gadget, "Endpoint Command Complete");
 		break;
 	}
@@ -2278,7 +2344,7 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum, bool force)
 	dep->flags &= ~DWC3_EP_BUSY;
 
 	if (dwc3_is_usb31(dwc) || dwc->revision < DWC3_REVISION_310A)
-		udelay(100);
+		dep->flags |= DWC3_EP_CMDCMPLT_BUSY;
 }
 
 static void dwc3_stop_active_transfers(struct dwc3 *dwc)
@@ -2936,6 +3002,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	}
 
 	init_completion(&dwc->ep0_in_setup);
+	init_completion(&dwc->cmd_complete);
 
 	dwc->gadget.ops			= &dwc3_gadget_ops;
 	dwc->gadget.speed		= USB_SPEED_UNKNOWN;
-- 
1.7.9.5

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ