Message-ID: <e2ec66ae-2516-4030-8bb2-51ed5c8918db@rowland.harvard.edu>
Date: Mon, 22 Dec 2025 22:24:35 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Michal Pecio <michal.pecio@...il.com>
Cc: 胡连勤 <hulianqin@...o.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Lee Jones <lee@...nel.org>,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: xhci: check Null pointer in segment alloc

[Redundant and invalid entries removed from CC: list]

On Mon, Dec 22, 2025 at 10:03:49PM +0100, Michal Pecio wrote:
> On Mon, 22 Dec 2025 12:03:45 -0500, Alan Stern wrote:
> > There's not supposed to be an inappropriate time for doing an 
> > autoresume.
> 
> > By the time the sound device's resume routine runs, the HC should be 
> > fully resumed.
> 
> OK, if "should" means "supposed to" then somebody needs to check it.
> Is this the HCD_FLAG_HW_ACCESSIBLE flag by any chance?

HCD_FLAG_HW_ACCESSIBLE means the HC is powered and can respond to I/O.  
If the flag is clear, it means the HC is unpowered or in a low-power 
mode or dead, and I/O accesses are likely to cause some sort of hardware 
exception.  At a minimum, they probably won't work.  (Think of a PCI HC 
in D3.  It won't respond to PCI traffic to any but a couple of its 
registers until it is put back into D0.)

> I see that devices recursively call bus_resume() before resuming, and

Are you talking about hcd_bus_resume()?  (There is no function named 
bus_resume() in usbcore.)  That's the routine in charge of resuming root 
hubs.  The PM core ensures that all of a device's ancestors are at full 
power before the device is resumed, so it would (indirectly) call this 
routine if an entire USB bus was suspended when a resume was requested 
for one of the devices on that bus.

I can't see it being an autoresume in that situation, though.  An 
autoresume is one that was requested by the device itself -- a wakeup 
request -- and that can't happen if the HC is suspended.  First there 
would be an autoresume of the HC, and only when that was finished and 
the hub driver started checking the status of the HC's ports would the 
device's wakeup request be noticed and acted on.

> this fails with -ESHUTDOWN if the flag is unset, which seems to prevent
> device resume from progressing further and crashing. Is this what is
> meant to happen in such case?

I'm not sure what code in what routine you're talking about.  However, 
it's obvious that if the host controller's registers can't be accessed 
then no downstream device resume is going to work.  On the other hand, 
the result should just be a normal failure, with an error code return -- 
not a crash.

> So I guess it's not happening because xhci_resume() sets this flag
> right away and then it may drop the lock and start deallocating memory
> to reset everything. So we can "successfully" complete bus_resume()
> and allow USB devices to resume while HC resume is still in progress.

The root-hub resume (i.e., the ->bus_resume() callback) does not occur 
until after the HC's own resume handler returns.

> Looks dodgy and I suspect this is the bug.

No, there should never be a device resume while the HC resume is still 
in progress.  If one happens, it means there must be a bug somewhere.

Is it possible that the HC's resume handler decided that the HC was 
dead, and so started deallocating stuff, but failed to call 
usb_hc_died()?  (But note that resume_common() in hcd-pci.c calls 
usb_hc_died() automatically on the HCD's behalf when a resume fails.)

Alan Stern

Powered by blists - more mailing lists