答复: [PATCH] xhci: sideband: check vdev liveness before removing endpoints on unregister

[胡连勤]

Hi Mathias,

> >
> > But xhci_discover_or_reset_device() is called: before hub_port_init()
> > calls problematic hub_enable_device() / hub_address_device() functions,
> > it calls hub_port_reset(), which calls hcd->driver->reset_device().
> 
> To me it looks like both drv->pre_reset and xhci_discover_or_reset_device()
> are called in this path.
> 
Your code tracing is correct. drv->pre_reset() IS called at
hub.c:6412 before usb_reset_and_verify_device(), and
xhci_discover_or_reset_device() IS called via hub_port_reset() →
hcd->driver->reset_device() inside hub_port_init().

However, this path is not the actual crash path. I apologize —
my earlier call chain referencing usb_reset_device() was an
assumption, not from the actual crash dump.


> hub_event()
>    port_event()
>      usb_reset_device(udev)
>        if (config) //and for each interface in this config: , for each interface)
>          if (cintf->dev.driver) {
>            drv = to_usb_driver(cintf->dev.driver);
>            if (drv->pre_reset && drv->post_reset)
>            unbind = (drv->pre_reset)(cintf);
> 
> 
> Any chance you could trace the whole call path in more details and see exactly which path
> is staken before the crash.
> 
> port_event() should only call usb_reset_device() for usb3 devices with link stuck in
> ss.inactive for longer than ~100ms.  Is this really a usb3 audio device?
>
No, it is not. The device is a full-speed Apple EarPods
(USB 1.1):

  usb 1-1: new full-speed USB device number 2 using xhci-hcd
  usb 1-1: Product: EarPods
  usb 1-1: New USB device found, idVendor=05ac, idProduct=110b

So the port_event() → usb_reset_device() warm-reset path for
SS.Inactive does not apply here.
The actual crash path is through usb_disconnect(), not
usb_reset_device(). The full sequence from dmesg + crash trace:

1. xhci_setup_device() gets COMP_USB_TRANSACTION_ERROR
("Device not responding to setup address")

2. xhci_disable_and_free_slot() → xhci_free_virt_device()
frees the entire vdev struct (subsequently SLAB-poisoned),
but does NOT clear sb->vdev

3. Later: usb_disconnect() → usb_audio_disconnect()
→ uaudio_disconnect() → xhci_sideband_unregister()
→ xhci_stop_endpoint_sync() → xhci_get_ep_ctx()
← CRASH: dereferences vdev->out_ctx, reads SLAB poison
value 0xdead000000000122
(0x100 = SLAB_POISON base, 0x22 = out_ctx field offset)

Crash call trace:
  xhci_get_ep_ctx+0x0/0x38
  xhci_sideband_unregister+0x68/0xf0
  uaudio_disconnect+0x70/0x144
  usb_audio_disconnect+0x7c/0x268
  usb_unbind_interface+0x13c/0x340
  device_release_driver_internal+0x1c4/0x2bc
  device_release_driver+0x18/0x28
  bus_remove_device+0x158/0x170
  device_del+0x1c8/0x320
  usb_disable_device+0x84/0x190
  usb_disconnect+0xe8/0x338
  hub_event+0xbd8/0x19ac

So both drv->pre_reset and xhci_discover_or_reset_device() are
called as you traced, but they are on the reset path — the crash
happens on the disconnect path.

The root cause is that xhci_free_virt_device() frees vdev without
clearing the sideband's dangling pointer. The fix (patch
2482d78a7813) clears sb->vdev = NULL in xhci_free_virt_device()
before freeing vdev, so xhci_sideband_unregister() can detect the
already-freed vdev and skip endpoint cleanup.

Regards,
Lianqin