All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] xhci: Identify USB 3.1 capable hosts by their port protocol capability
       [not found] <1507301130-11022-1-git-send-email-mathias.nyman@linux.intel.com>
@ 2017-10-06 14:45 ` Mathias Nyman
  2017-10-06 14:45 ` [PATCH 2/4] xhci: Cleanup current_cmd in xhci_cleanup_command_queue() Mathias Nyman
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2017-10-06 14:45 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman, # v4 . 6+

Many USB 3.1 capable hosts never updated the Serial Bus Release Number
(SBRN) register to USB 3.1 from USB 3.0

xhci driver identified USB 3.1 capable hosts based on this SBRN register,
which according to specs "contains the release of the Universal Serial
Bus Specification with which this Universal Serial Bus Host Controller
module is compliant." but still in october 2017 gives USB 3.0 as
the only possible option.

Make an additional check for USB 3.1 support and enable it if the xHCI
supported protocol capablity lists USB 3.1 capable ports.

Cc: <stable@vger.kernel.org> # v4.6+
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ee198ea..51535ba 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4805,7 +4805,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 		 */
 		hcd->has_tt = 1;
 	} else {
-		if (xhci->sbrn == 0x31) {
+		/* Some 3.1 hosts return sbrn 0x30, can't rely on sbrn alone */
+		if (xhci->sbrn == 0x31 || xhci->usb3_rhub.min_rev >= 1) {
 			xhci_info(xhci, "Host supports USB 3.1 Enhanced SuperSpeed\n");
 			hcd->speed = HCD_USB31;
 			hcd->self.root_hub->speed = USB_SPEED_SUPER_PLUS;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/4] xhci: Cleanup current_cmd in xhci_cleanup_command_queue()
       [not found] <1507301130-11022-1-git-send-email-mathias.nyman@linux.intel.com>
  2017-10-06 14:45 ` [PATCH 1/4] xhci: Identify USB 3.1 capable hosts by their port protocol capability Mathias Nyman
@ 2017-10-06 14:45 ` Mathias Nyman
  2017-10-06 14:45 ` [PATCH 3/4] usb: xhci: Reset halted endpoint if trb is noop Mathias Nyman
  2017-10-06 14:45 ` [PATCH 4/4] usb: xhci: Handle error condition in xhci_stop_device() Mathias Nyman
  3 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2017-10-06 14:45 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Jeffy Chen, # v4 . 12+, Mathias Nyman

From: Jeffy Chen <jeffy.chen@rock-chips.com>

KASAN reported use-after-free bug when xhci host controller died:
[  176.952537] BUG: KASAN: use-after-free in xhci_handle_command_timeout+0x68/0x224
[  176.960846] Write of size 4 at addr ffffffc0cbb01608 by task kworker/3:3/1680
...
[  177.180644] Freed by task 0:
[  177.183882]  kasan_slab_free+0x90/0x15c
[  177.188194]  kfree+0x114/0x28c
[  177.191630]  xhci_cleanup_command_queue+0xc8/0xf8
[  177.196916]  xhci_hc_died+0x84/0x358

Problem here is that when the cmd_timer fired, it would try to access
current_cmd while the command queue is already freed by xhci_hc_died().

Cleanup current_cmd in xhci_cleanup_command_queue() to avoid that.

Fixes: d9f11ba9f107 ("xhci: Rework how we handle unresponsive or hoptlug removed hosts")
Cc: <stable@vger.kernel.org> # v4.12+
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a944365..48ae15af 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1309,6 +1309,7 @@ static void xhci_complete_del_and_free_cmd(struct xhci_command *cmd, u32 status)
 void xhci_cleanup_command_queue(struct xhci_hcd *xhci)
 {
 	struct xhci_command *cur_cmd, *tmp_cmd;
+	xhci->current_cmd = NULL;
 	list_for_each_entry_safe(cur_cmd, tmp_cmd, &xhci->cmd_list, cmd_list)
 		xhci_complete_del_and_free_cmd(cur_cmd, COMP_COMMAND_ABORTED);
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/4] usb: xhci: Reset halted endpoint if trb is noop
       [not found] <1507301130-11022-1-git-send-email-mathias.nyman@linux.intel.com>
  2017-10-06 14:45 ` [PATCH 1/4] xhci: Identify USB 3.1 capable hosts by their port protocol capability Mathias Nyman
  2017-10-06 14:45 ` [PATCH 2/4] xhci: Cleanup current_cmd in xhci_cleanup_command_queue() Mathias Nyman
@ 2017-10-06 14:45 ` Mathias Nyman
  2017-10-06 14:45 ` [PATCH 4/4] usb: xhci: Handle error condition in xhci_stop_device() Mathias Nyman
  3 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2017-10-06 14:45 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Lu Baolu, stable, Mathias Nyman

From: Lu Baolu <baolu.lu@linux.intel.com>

When a URB is cancled, xhci driver turns the untransferred trbs
into no-ops.  If an endpoint stalls on a no-op trb that belongs
to the cancelled URB, the event handler won't reset the endpoint.
Hence, it will stay halted.

Link: http://marc.info/?l=linux-usb&m=149582598330127&w=2

Cc: <stable@vger.kernel.org>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 48ae15af..82c746e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2580,15 +2580,21 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 				(struct xhci_generic_trb *) ep_trb);
 
 		/*
-		 * No-op TRB should not trigger interrupts.
-		 * If ep_trb is a no-op TRB, it means the
-		 * corresponding TD has been cancelled. Just ignore
-		 * the TD.
+		 * No-op TRB could trigger interrupts in a case where
+		 * a URB was killed and a STALL_ERROR happens right
+		 * after the endpoint ring stopped. Reset the halted
+		 * endpoint. Otherwise, the endpoint remains stalled
+		 * indefinitely.
 		 */
 		if (trb_is_noop(ep_trb)) {
-			xhci_dbg(xhci,
-				 "ep_trb is a no-op TRB. Skip it for slot %u ep %u\n",
-				 slot_id, ep_index);
+			if (trb_comp_code == COMP_STALL_ERROR ||
+			    xhci_requires_manual_halt_cleanup(xhci, ep_ctx,
+							      trb_comp_code))
+				xhci_cleanup_halted_endpoint(xhci, slot_id,
+							     ep_index,
+							     ep_ring->stream_id,
+							     td, ep_trb,
+							     EP_HARD_RESET);
 			goto cleanup;
 		}
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 4/4] usb: xhci: Handle error condition in xhci_stop_device()
       [not found] <1507301130-11022-1-git-send-email-mathias.nyman@linux.intel.com>
                   ` (2 preceding siblings ...)
  2017-10-06 14:45 ` [PATCH 3/4] usb: xhci: Reset halted endpoint if trb is noop Mathias Nyman
@ 2017-10-06 14:45 ` Mathias Nyman
  3 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2017-10-06 14:45 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mayank Rana, stable, Jack Pham, Mathias Nyman

From: Mayank Rana <mrana@codeaurora.org>

xhci_stop_device() calls xhci_queue_stop_endpoint() multiple times
without checking the return value. xhci_queue_stop_endpoint() can
return error if the HC is already halted or unable to queue commands.
This can cause a deadlock condition as xhci_stop_device() would
end up waiting indefinitely for a completion for the command that
didn't get queued. Fix this by checking the return value and bailing
out of xhci_stop_device() in case of error. This patch happens to fix
potential memory leaks of the allocated command structures as well.

Fixes: c311e391a7ef ("xhci: rework command timeout and cancellation,")
Cc: <stable@vger.kernel.org>
Signed-off-by: Mayank Rana <mrana@codeaurora.org>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index da9158f..a2336de 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -420,14 +420,25 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 						     GFP_NOWAIT);
 			if (!command) {
 				spin_unlock_irqrestore(&xhci->lock, flags);
-				xhci_free_command(xhci, cmd);
-				return -ENOMEM;
+				ret = -ENOMEM;
+				goto cmd_cleanup;
+			}
+
+			ret = xhci_queue_stop_endpoint(xhci, command, slot_id,
+						       i, suspend);
+			if (ret) {
+				spin_unlock_irqrestore(&xhci->lock, flags);
+				xhci_free_command(xhci, command);
+				goto cmd_cleanup;
 			}
-			xhci_queue_stop_endpoint(xhci, command, slot_id, i,
-						 suspend);
 		}
 	}
-	xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+	ret = xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+	if (ret) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		goto cmd_cleanup;
+	}
+
 	xhci_ring_cmd_db(xhci);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -439,6 +450,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 		xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n");
 		ret = -ETIME;
 	}
+
+cmd_cleanup:
 	xhci_free_command(xhci, cmd);
 	return ret;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-06 14:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1507301130-11022-1-git-send-email-mathias.nyman@linux.intel.com>
2017-10-06 14:45 ` [PATCH 1/4] xhci: Identify USB 3.1 capable hosts by their port protocol capability Mathias Nyman
2017-10-06 14:45 ` [PATCH 2/4] xhci: Cleanup current_cmd in xhci_cleanup_command_queue() Mathias Nyman
2017-10-06 14:45 ` [PATCH 3/4] usb: xhci: Reset halted endpoint if trb is noop Mathias Nyman
2017-10-06 14:45 ` [PATCH 4/4] usb: xhci: Handle error condition in xhci_stop_device() Mathias Nyman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.