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]
Message-Id: <20240912062606.357373-1-duanchenghao@kylinos.cn>
Date: Thu, 12 Sep 2024 14:26:06 +0800
From: Duan Chenghao <duanchenghao@...inos.cn>
To: duanchenghao@...inos.cn
Cc: gregkh@...uxfoundation.org,
	linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org,
	linux-usb@...r.kernel.org,
	niko.mauno@...sala.com,
	pavel@....cz,
	stanley_chang@...ltek.com,
	stern@...land.harvard.edu,
	tj@...nel.org
Subject: [PATCH v2] USB: Fix the issue of task recovery failure caused by USB status when S4 wakes up

When a device is inserted into the USB port and an S4 wakeup is initiated,
after the USB-hub initialization is completed, it will automatically enter
suspend mode. Upon detecting a device on the USB port, it will proceed with
resume and set the hcd to the HCD_FLAG_WAKEUP_PENDING state. During the S4
wakeup process, peripherals are put into suspend mode, followed by task
recovery. However, upon detecting that the hcd is in the
HCD_FLAG_WAKEUP_PENDING state, it will return an EBUSY status, causing the
S4 suspend to fail and subsequent task recovery to not proceed.

    S4 waking up from hibernation
    =============================
    kernel initialization
    |
    v
    freeze user task and kernel thread
    |
    v
    load saved image
    |
    v
    freeze the peripheral device and controller(*** Error point ***)
    (Check the HCD_FLAG_WAKEUP_ PENDING flag of the USB. If it is set,
     return to EBUSY and do not perform the following restore image.)
    |
    v
    restore image(task recovery)

This patch makes two modifications in total:
1. The set_bit and clear_bit operations for the HCD_FLAG_WAKEUP_PENDING
flag of Hcd, which were previously split between the top half and bottom
half of the interrupt,are now unified and executed solely in the bottom
half of the interrupt.This prevents the bottom half tasks from being frozen
during the S4 process,ensuring that the clear_bit process can proceed
without interruption.

    Before modification:

    ehci interrupt handler
    =====================
    remote wakeup
    if (ehci->rh_state == EHCI_RH_SUSPENDED)	    |
	usb_hcd_resume_root_hub			    |
    |						    |
    v						    | top half
    set_bit HCD_FLAG_WAKEUP_PENDING		    |
    |						    |
    v						    |
    queue_work(pm_wq, &hcd->wakeup_work)	    |
    |
    v
    hub resume					    |
    |						    |
    v						    | bottom half
    clear_bit HCD_FLAG_WAKEUP_PENDING		    |

    After modification:

    ehci interrupt handler
    =====================
    remote wakeup
    if (ehci->rh_state == EHCI_RH_SUSPENDED)        |
        usb_hcd_resume_root_hub                     |
    |						    | top half
    v						    |
    queue_work(pm_wq, &hcd->wakeup_work)	    |
    |
    v
    set_bit HCD_FLAG_WAKEUP_PENDING		    |
    |						    |
    v						    |
    hub resume                                      | bottom half
    |                                               |
    v                                               |
    clear_bit HCD_FLAG_WAKEUP_PENDING               |

2. Add a condition to the set_bit operation for the hcd flags
HCD_FLAG_WAKEUP_PENDING.When the hcd status is HC_STATE_SUSPENDED, perform
the setting of the aforementioned status bit.This prevents a subsequent
set_bit from occurring after the clear_bit if the hcd is in the resuming
process.

    Before modification:
    ehci interrupt handler(CPUx)		ehci init(CPUy)
    =====================			=============
    remote wakeup				initialization complete
    usb_hcd_resume_root_hub
    				    		Enter suspend and detect that there
						is a device inserted
						(udev->state = USB_STATE_SUSPENDED)

						hcd->state = HC_STATE_RESUMING
						bus_resume
    if (udev->state == USB_STATE_SUSPENDED)	clear_bit HCD_FLAG_WAKEUP_PENDING
    set_bit HCD_FLAG_WAKEUP_PENDING		udev->state = !USB_STATE_SUSPENDED

    In the aforementioned scenario, there is no mutual exclusion between
    the set_bit operation in ehci interrupts, the judgment of udev->state,
    and the clear_bit as well as the assignment of udev->state during ehci
    initialization. There exists a situation where clear_bit is followed by
    set_bit, but due to the judgment on udev->state, the clear_bit
    operation might not occur again, leading to an error in the flag bit.

    After modification:
    ehci interrupt handler(CPUx)		ehci init(CPUy)
    =====================			=============
    remote wakeup				initialization complete
    usb_hcd_resume_root_hub
    				    		Enter suspend and detect that there
						is a device inserted
						(udev->state = USB_STATE_SUSPENDED)

						hcd->state = HC_STATE_RESUMING
						bus_resume
    if (udev->state == USB_STATE_SUSPENDED)	clear_bit HCD_FLAG_WAKEUP_PENDING
    if (hcd->state != HC_STATE_RESUMING)	udev->state = !USB_STATE_SUSPENDED
    set_bit HCD_FLAG_WAKEUP_PENDING

Signed-off-by: Duan Chenghao <duanchenghao@...inos.cn>
---
 drivers/usb/core/hcd.c | 1 -
 drivers/usb/core/hub.c | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 1ff7d901fede..a6bd0fbd82f4 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2389,7 +2389,6 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->rh_registered) {
 		pm_wakeup_event(&hcd->self.root_hub->dev, 0);
-		set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
 		queue_work(pm_wq, &hcd->wakeup_work);
 	}
 	spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 4b93c0bd1d4b..7f847c4afc0d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3835,11 +3835,14 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 
 int usb_remote_wakeup(struct usb_device *udev)
 {
+	struct usb_hcd  *hcd = bus_to_hcd(udev->bus);
 	int	status = 0;
 
 	usb_lock_device(udev);
 	if (udev->state == USB_STATE_SUSPENDED) {
 		dev_dbg(&udev->dev, "usb %sresume\n", "wakeup-");
+		if (hcd->state == HC_STATE_SUSPENDED)
+			set_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
 		status = usb_autoresume_device(udev);
 		if (status == 0) {
 			/* Let the drivers do their thing, then... */
-- 
2.34.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ