linux-kernel - Re: [PATCH] USB: Fix the issue of task recovery failure caused by USB status when S4 wakes up

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: <bddecd4e-d3c8-448e-8a22-84bbc98c4d1b@kylinos.cn>
Date: Sat, 14 Sep 2024 10:43:12 +0800
From: Hongyu Xie <xy521521@...il.com>
To: stern@...land.harvard.edu,
	duanchenghao <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,
	tj@...nel.org,
	Hongyu Xie <xiehongyu1@...inos.cn>
Subject: Re: [PATCH] USB: Fix the issue of task recovery failure caused by USB
 status when S4 wakes up

From: Hongyu Xie <xiehongyu1@...inos.cn>


Hi Alan,
On 2024/9/12 23:00, Alan Stern wrote:
> On Thu, Sep 12, 2024 at 11:21:26AM +0800, duanchenghao wrote:
>> 在 2024-09-11星期三的 10:40 -0400，Alan Stern写道：
>>> On Tue, Sep 10, 2024 at 05:36:56PM +0800, duanchenghao wrote:
>>>> S4 wakeup restores the image that was saved before the system
>>>> entered
>>>> the S4 sleep state.
>>>>
>>>>      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
>>>>      (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.)
>>>
>>> Why is the flag set at this point?  It should not be; the device and
>>> controller should have been frozen with wakeup disabled.
>>>
>> This is check point, not set point.
> 
> Yes, I know that.  But when the flag was checked, why did the code find
> that it was set?  The flag should have been clear.
Maybe duanchenghao means this,
freeze_kernel_threads
load_image_and_restore
   suspend roothub
                                      interrupt occurred
                                        usb_hcd_resume_root_hub
                                          set HCD_FLAG_WAKEUP_PENDING
                                          queue_work // freezed
   suspend pci
     return -EBUSY  because HCD_FLAG_WAKEUP_PENDING

So s4 resume failed.
> 
>>> Is your problem related to the one discussed in this email thread?
>>>
>>> https://lore.kernel.org/linux-usb/d8600868-6e4b-45ab-b328-852b6ac8ecb5@rowland.harvard.edu/
>>>
>>> Would the suggestion I made there -- i.e., have the xhci-hcd
>>> interrupt handler skip calling usb_hcd_resume_root_hub() if the root
>>> hub
>>> was suspended with wakeup = 0 -- fix your problem?
>>
>> Skipping usb_hcd_resume_root_hub() should generally be possible, but
>> it's important to ensure that normal remote wakeup functionality is not
>> compromised. Is it HUB_SUSPEND that the hub you are referring to is in
>> a suspended state?
> 
> I don't understand this question.  hub_quiesce() gets called with
> HUB_SUSPEND when the hub enters a suspended state.
> 
> You are correct about the need for normal remote wakeup to work
> properly.  The interrupt handler should skip calling
> usb_hcd_resume_root_hub() for port connect or disconnect changes and for
> port overcurrent changes (when the root hub is suspended with wakeup =
> 0).  But it should _not_ skip calling usb_hcd_resume_root_hub() for port
> resume events.
> 
> Alan Stern
> 

Hongyu Xie