Re: 答复: 答复: [PATCH] xhci: sideband: check vdev liveness before removing endpoints on unregister
From: Mathias Nyman
Date: Fri Sep 11 2026 - 09:27:13 EST
On 9/11/26 11:41, Selvarasu Ganesan wrote:
On 9/11/2026 12:59 PM, 胡连勤 wrote:
Hi Selva,
Include the corresponding header filedrivers/usb/host/xhci-mem.c | 8 ++++++++Thanks for your updated patch.
drivers/usb/host/xhci-sideband.c | 25 ++++++++++++++++++-------
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index af8d4b74c4ba..afdcfb38b35f 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -922,6 +922,14 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
dev->rhub_port->slot_id = 0;
if (xhci->devs[slot_id] == dev)
xhci->devs[slot_id] = NULL;
+
+ if (dev->sideband) {
+ xhci_dbg(xhci, "vdev for slot %d has sideband still set at free, clearing dangling pointer\n",
+ slot_id);
+ dev->sideband->vdev = NULL;
Dont forget to add #include <linux/usb/xhci-sideband.h> in this
xhci-mem.c file otherwise getting below error,
drivers/usb/host/xhci-mem.c:928:30: error: invalid use of undefined type
‘struct xhci_sideband’
928 | dev->sideband->vdev = NULL;
#include <linux/dmapool.h>
#include <linux/dma-mapping.h>
#include <linux/bitfield.h>
+#include <linux/usb/xhci-sideband.h>
+ dev->sideband = NULL;I have one query on skip endpoint cleanup due to vdev is NULL, in this
+ }
+
kfree(dev);
}
diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index a5deeee4d5dc..6312c9e3af65 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -472,12 +472,22 @@ xhci_sideband_unregister(struct xhci_sideband *sb)
scoped_guard(mutex, &sb->mutex) {
vdev = sb->vdev;
- if (!vdev)
- return;
-
- for (i = 0; i < EP_CTX_PER_DEV; i++)
- if (sb->eps[i])
- __xhci_sideband_remove_endpoint(sb, sb->eps[i]);
+ /*
+ * If vdev is NULL, xhci_free_virt_device() has already
+ * cleared sb->vdev and freed vdev (e.g. on
+ * COMP_USB_TRANSACTION_ERROR during address device
+ * recovery). Skip endpoint cleanup as the xHC has already
+ * disabled the slot.
+ *
+ * The interrupter and sideband instance are host-level
+ * resources independent of vdev, so still remove and free
+ * them to avoid leaks.
+ */
+ if (vdev) {
+ for (i = 0; i < EP_CTX_PER_DEV; i++)
+ if (sb->eps[i])
+ __xhci_sideband_remove_endpoint(sb, sb->eps[i]);
case the pointers in sb->eps are not cleared. Since these pointers point
into the virtual device eps, any subsequent call to sideband API
functions like xhci_sideband_get_endpoint_buffer() that dereference
sb->eps could result in a use after free if the virtual device has been
freed. Is it possible?
Good point, endpoint use after free is a much bigger and earlier issue here.
The endpoint rings that audio driver is accessing via sideband are freed and
reallocated much earlier. Audio driver is unaware of this reset, and may still
try to access the freed ring buffers.
This is an issue long before xhci_sideband_unregister() is called.
usb_reset_and_verify_device()
hub_port_init() // resets port
usb_hcd_alloc_bandwidth(udev, udev->actconfig, NULL, NULL);
hcd->driver->drop_endpoint() // for all endpoints, xhci tags ep to be dropped
hcd->driver->add_endpoint() // for active endpoints. xhci allocs new ring for ep
hcd->driver->check_bandwidth(hcd, udev) // xhci frees old ring and takes new ring into use
So turns out setting vdev->sideband->vdev to NULL in xhci_free_virt_dev(), and
reacting to it in xhci_sideband_unregister() is too little too late.
I think wee need to look at using drv->pre_reset and drv->post_reset
to unregister and re-register sideband, or optionally to unbind and rebind
the whole interface.
Thanks
Mathias