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: <b2ec107d4797f6e1e8e558f97c0ad1be6d46572c.camel@kylinos.cn>
Date: Mon, 23 Sep 2024 16:00:35 +0800
From: duanchenghao <duanchenghao@...inos.cn>
To: Hongyu Xie <xy521521@...il.com>, stern@...land.harvard.edu
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

Hi Alan,

Do you think this plan is feasible, or is there any unclear part in my
description that needs to be supplemented?

duanchenghao


在 2024-09-14星期六的 10:43 +0800，Hongyu Xie写道：
> 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