linux-kernel - [RFT PATCH] xhci: Manually give back cancelled URBs if queuing for cancel fails

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Mar 2017 15:12:01 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     diego.viola@...il.com
Cc:     rogerable@...ltek.com, ulf.hansson@...aro.org,
        gregkh@...uxfoundation.org, wei_wang@...lsil.com.cn,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        stern@...land.harvard.edu,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        stable@...r.kernel.org
Subject: [RFT PATCH] xhci: Manually give back cancelled URBs if queuing for cancel fails

Manually give back URB if we are not able to add it to cancel queue and
stop the endpoint normally.
This can happen if device just reset before URB timed out and dequeued,
leading to missing endpoint ring.

This caused a hang on Dell Inspiron 5558/0VNM2T at resume from suspend
as urb was never returned.

[  245.270505] INFO: task rtsx_usb_ms_1:254 blocked for more than 120 seconds.
[  245.272244]       Tainted: G        W       4.11.0-rc3-ARCH #2
[  245.273983] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  245.275737] rtsx_usb_ms_1   D    0   254      2 0x00000000
[  245.277524] Call Trace:
[  245.279278]  __schedule+0x2d3/0x8a0
[  245.281077]  schedule+0x3d/0x90
[  245.281961]  usb_kill_urb.part.3+0x6c/0xa0 [usbcore]
[  245.282861]  ? wake_atomic_t_function+0x60/0x60
[  245.283760]  usb_kill_urb+0x21/0x30 [usbcore]
[  245.284649]  usb_start_wait_urb+0xe5/0x170 [usbcore]
[  245.285541]  ? try_to_del_timer_sync+0x53/0x80
[  245.286434]  usb_bulk_msg+0xbd/0x160 [usbcore]
[  245.287326]  rtsx_usb_send_cmd+0x63/0x90 [rtsx_usb]

Reported-by: diego.viola@...il.com
Cc: stable@...r.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
---
 drivers/usb/host/xhci.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 50aee8b..953fd8f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1477,6 +1477,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 	struct xhci_ring *ep_ring;
 	struct xhci_virt_ep *ep;
 	struct xhci_command *command;
+	struct xhci_virt_device *vdev;
 
 	xhci = hcd_to_xhci(hcd);
 	spin_lock_irqsave(&xhci->lock, flags);
@@ -1485,15 +1486,27 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 
 	/* Make sure the URB hasn't completed or been unlinked already */
 	ret = usb_hcd_check_unlink_urb(hcd, urb, status);
-	if (ret || !urb->hcpriv)
+	if (ret)
 		goto done;
+
+	/* give back URB now if we can't queue it for cancel */
+	vdev = xhci->devs[urb->dev->slot_id];
+	urb_priv = urb->hcpriv;
+	if (!vdev || !urb_priv)
+		goto err_giveback;
+
+	ep_index = xhci_get_endpoint_index(&urb->ep->desc);
+	ep = &vdev->eps[ep_index];
+	ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
+	if (!ep || !ep_ring)
+		goto err_giveback;
+
 	temp = readl(&xhci->op_regs->status);
 	if (temp == 0xffffffff || (xhci->xhc_state & XHCI_STATE_HALTED)) {
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
 				"HW died, freeing TD.");
-		urb_priv = urb->hcpriv;
 		for (i = urb_priv->num_tds_done;
-		     i < urb_priv->num_tds && xhci->devs[urb->dev->slot_id];
+		     i < urb_priv->num_tds;
 		     i++) {
 			td = &urb_priv->td[i];
 			if (!list_empty(&td->td_list))
@@ -1501,23 +1514,9 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 			if (!list_empty(&td->cancelled_td_list))
 				list_del_init(&td->cancelled_td_list);
 		}
-
-		usb_hcd_unlink_urb_from_ep(hcd, urb);
-		spin_unlock_irqrestore(&xhci->lock, flags);
-		usb_hcd_giveback_urb(hcd, urb, -ESHUTDOWN);
-		xhci_urb_free_priv(urb_priv);
-		return ret;
+		goto err_giveback;
 	}
 
-	ep_index = xhci_get_endpoint_index(&urb->ep->desc);
-	ep = &xhci->devs[urb->dev->slot_id]->eps[ep_index];
-	ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
-	if (!ep_ring) {
-		ret = -EINVAL;
-		goto done;
-	}
-
-	urb_priv = urb->hcpriv;
 	i = urb_priv->num_tds_done;
 	if (i < urb_priv->num_tds)
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -1554,6 +1553,14 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 done:
 	spin_unlock_irqrestore(&xhci->lock, flags);
 	return ret;
+
+err_giveback:
+	if (urb_priv)
+		xhci_urb_free_priv(urb_priv);
+	usb_hcd_unlink_urb_from_ep(hcd, urb);
+	spin_unlock_irqrestore(&xhci->lock, flags);
+	usb_hcd_giveback_urb(hcd, urb, -ESHUTDOWN);
+	return ret;
 }
 
 /* Drop an endpoint from a new bandwidth configuration for this device.
-- 
1.9.1