linux-kernel - [V2] usb:xhci: Avoid hub_event() stuck when xHC restore state timeout

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

Message-ID: <TYZPR01MB4784EC31D8A4BC381AD83451D58DA@TYZPR01MB4784.apcprd01.prod.exchangelabs.com>
Date:   Wed, 13 Dec 2023 14:18:38 +0800
From:   Yaxiong Tian <iambestgod@...look.com>
To:     mathias.nyman@...el.com, gregkh@...uxfoundation.org
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Yaxiong Tian <tianyaxiong@...inos.cn>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        Hongyu Xie <xiehongyu1@...inos.cn>
Subject: [V2] usb:xhci: Avoid hub_event() stuck when xHC restore state timeout

From: Yaxiong Tian <tianyaxiong@...inos.cn>

when xHc restore state timeout,the xhci_reusme() return -ETIMEDOUT
instantly. After usb_hc_died() called ,they kick hub_wq to running
hub_event() but the wq is freezd. When suspend ends,hub_evnet realy
running and sticking.
Such as:
[  968.794016][ 2] [   T37] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[  968.802969][ 2] [   T37] kworker/2:3     D    0   999      2 0x00000028
[  968.809579][ 2] [   T37] Workqueue: usb_hub_wq hub_event
[  968.814885][ 2] [   T37] Call trace:
[  968.818455][ 2] [   T37]  __switch_to+0xd4/0x138
[  968.823067][ 2] [   T37]  __schedule+0x2dc/0x6a0
[  968.827680][ 2] [   T37]  schedule+0x34/0xb0
[  968.831947][ 2] [   T37]  schedule_timeout+0x1e0/0x298
[  968.837079][ 2] [   T37]  __wait_for_common+0xf0/0x208
[  968.842212][ 2] [   T37]  wait_for_completion+0x1c/0x28
[  968.847432][ 2] [   T37]  xhci_configure_endpoint+0x104/0x640
[  968.853173][ 2] [   T37]  xhci_check_bandwidth+0x140/0x2e0
[  968.858652][ 2] [   T37]  usb_hcd_alloc_bandwidth+0x1c8/0x348
[  968.864393][ 2] [   T37]  usb_disable_device+0x198/0x260
[  968.869698][ 2] [   T37]  usb_disconnect+0xdc/0x3a0
[  968.874571][ 2] [   T37]  usb_disconnect+0xbc/0x3a0
[  968.879441][ 2] [   T37]  hub_quiesce+0xa0/0x108
[  968.884053][ 2] [   T37]  hub_event+0x4d4/0x1558
[  968.888664][ 2] [   T37]  kretprobe_trampoline+0x0/0xc4
[  968.893884][ 2] [   T37]  worker_thread+0x4c/0x488
[  968.898668][ 2] [   T37]  kthread+0xf8/0x128
[  968.902933][ 2] [   T37]  ret_from_fork+0x10/0x18

The result is that you cannot suspend again.because the wq can't
be freezed.Also hard to reboot,when some application visited this
piece.

The reason of stuck is that the xhci_resume() don't complete correctly.
In the error path before command |= CMD_RUN,the xhc should not be accessed.

To fix it, move set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags) and others
after "done:" .Also add HCD_HW_ACCESSIBLE() checks in
queue_command().

Suggested-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
Signed-off-by: Yaxiong Tian <tianyaxiong@...inos.cn>
Signed-off-by: Hongyu Xie <xiehongyu1@...inos.cn>
---
 drivers/usb/host/xhci-ring.c |  7 +++++--
 drivers/usb/host/xhci.c      | 10 +++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2c1d614b3b0f..ee6e06590974 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -4263,10 +4263,13 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
 {
 	int reserved_trbs = xhci->cmd_ring_reserved_trbs;
 	int ret;
+	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
 
 	if ((xhci->xhc_state & XHCI_STATE_DYING) ||
-		(xhci->xhc_state & XHCI_STATE_HALTED)) {
-		xhci_dbg(xhci, "xHCI dying or halted, can't queue_command\n");
+		(xhci->xhc_state & XHCI_STATE_HALTED) ||
+		!HCD_HW_ACCESSIBLE(hcd)) {
+		xhci_dbg(xhci,
+			"xHCI dying or halted,or hcd hw can't accessible,can't queue_command\n");
 		return -ESHUTDOWN;
 	}
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 08fbabff23a0..7f2c2b76a51c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1006,10 +1006,6 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
 	    time_before(jiffies, xhci->usb3_rhub.bus_state.next_statechange))
 		msleep(100);
 
-	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-	if (xhci->shared_hcd)
-		set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
-
 	spin_lock_irq(&xhci->lock);
 
 	if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend)
@@ -1133,9 +1129,13 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
 
 	spin_unlock_irq(&xhci->lock);
 
+ done:
+	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	if (xhci->shared_hcd)
+		set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
+
 	xhci_dbc_resume(xhci);
 
- done:
 	if (retval == 0) {
 		/*
 		 * Resume roothubs only if there are pending events.
-- 
2.25.1