public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: 胡连勤 <hulianqin@vivo.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Lee Jones" <lee@kernel.org>,
	"Mathias Nyman" <mathias.nyman@linux.intel.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: xhci: check Null pointer in segment alloc
Date: Tue, 23 Dec 2025 11:06:21 +0100	[thread overview]
Message-ID: <20251223110621.2b53f63d.michal.pecio@gmail.com> (raw)
In-Reply-To: <e2ec66ae-2516-4030-8bb2-51ed5c8918db@rowland.harvard.edu>

On Mon, 22 Dec 2025 22:24:35 -0500, Alan Stern wrote:
> > I see that devices recursively call bus_resume() before resuming,
> 
> 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.

Yes, I mean this function and the bus_resume() method of the HCD,
which the function calls.

> 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.

Indeed, the crashing call stack looks like some driver manually
resuming a USB device.

[ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
[ 4021.987711][  T332]  usb_set_interface+0x144/0x510
[ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
[ 4021.987723][  T332]  usb_port_resume+0x580/0x700
[ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
[ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
[ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
[ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
[ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
[ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
[ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
[ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
[ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
[ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]

Before we got here, we should have attempted an hcd_bus_resume().
If xhci_hcd tracked its HW_ACCESSIBLE state better, that would have
failed and hopefully aborted device resume before it crashed.

> > 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.

If HW_ACCESSIBLE isn't set then xhci_bus_resume() returns -ESHUTDOWN.
It always returns zero otherwise.

So in the light of your explanation, the fact that xhci_resume() sets
HW_ACCESSIBLE before actually completing resume and thus allows root
hub resume to pretend to work, is obviously a bug.

> > 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.

If PM core is supposed to prevent it then this is getting weird.
If not, then I'm not sure what else can prevent it.

> 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.)

Hopefully not.

xhci->segment_pool is only modified by xhci_mem_cleanup() and by
xhci_mem_init() if allocation fails. And those functions are only
called at probe time, during HC resume and by hc_driver->stop().

I'm out of ideas without more logs. The xhci HW_ACCESSIBLE bug should
be fixed, but I'm not sure about correct ordering of setting this bit
wrt some calls done by xhci_resume(), so no patch from me.

Regards,
Michal



  reply	other threads:[~2025-12-23 10:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-19  7:18 [PATCH] usb: xhci: check Null pointer in segment alloc 胡连勤
2025-12-19 12:48 ` Mathias Nyman
2025-12-19 15:53   ` 答复: " 胡连勤
2025-12-20  8:08     ` Greg Kroah-Hartman
2025-12-20 11:34       ` 答复: " 胡连勤
2025-12-20 13:15     ` Michal Pecio
2025-12-21  5:48       ` 答复: " 胡连勤
2025-12-22  6:42       ` Lee Jones
2025-12-22  7:13         ` Greg Kroah-Hartman
2025-12-22  7:55           ` Michal Pecio
2025-12-22 12:21             ` 答复: " 胡连勤
2025-12-22 13:34               ` Alan Stern
2025-12-22 16:49                 ` Michal Pecio
2025-12-22 17:03                   ` Alan Stern
2025-12-22 21:03                     ` Michal Pecio
2025-12-23  3:24                       ` Alan Stern
2025-12-23 10:06                         ` Michal Pecio [this message]
2025-12-23 18:37                           ` Alan Stern
2025-12-23 19:43                             ` Michal Pecio
2025-12-22 14:00               ` 答复: " Greg Kroah-Hartman
2025-12-21 16:22 ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251223110621.2b53f63d.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hulianqin@vivo.com \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox