[PATCH v2] usb: xhci: clear dangling sideband pointer in xhci_free_virt_device()

[胡连勤]

xhci_free_virt_device() must not leave any dangling pointers.
If vdev->sideband is still set at this point then something is
wrong, e.g. the sideband client did not unregister before the
virtual device was freed. This can happen when
xhci_setup_device() gets COMP_USB_TRANSACTION_ERROR (device not
responding to setup address during bus reset recovery), causing
xhci_disable_and_free_slot() -> xhci_free_virt_device() to free
vdev before the sideband client has a chance to unregister.

  hub_event()
    xhci_setup_device()                  <-- COMP_USB_TRANSACTION_ERROR
    xhci_disable_and_free_slot()
      xhci_free_virt_device()
        kfree(out_ctx), kfree(vdev)
        xhci->devs[slot_id] = NULL
    ...
    usb_disconnect()
      uaudio_disconnect()
        xhci_sideband_unregister()
          xhci_stop_endpoint_sync()
            xhci_get_ep_ctx()            <-- CRASH (deref freed out_ctx)

Unable to handle kernel paging request at virtual address dead000000000122
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
 process_scheduled_works+0x200/0x9d8
 worker_thread+0x154/0x3b0
 kthread+0x11c/0x1a0

Fix this by clearing any remaining sideband pointer in
xhci_free_virt_device() before freeing vdev. If vdev->sideband is
still set, set vdev->sideband->vdev = NULL to break the dangling
pointer at the source.

Additionally, in xhci_sideband_unregister(), check sb->vdev before
issuing stop endpoint commands. If vdev is already NULL (cleared by
xhci_free_virt_device), skip endpoint cleanup as the xHC has already
disabled the slot, but still remove the interrupter and free the
sideband instance to avoid leaks.

Fixes: de66754e9f80 ("xhci: sideband: add initial api to register a secondary interrupter entity")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Lianqin Hu <hulianqin@xxxxxxxx>
---

Changes in v2:
 - Move fix to xhci_free_virt_device() per maintainer suggestion.
 - Use xhci_dbg per maintainer suggestion.
 - Ensure interrupter cleanup is unconditional when vdev is NULL.
 - Clear dangling sb->eps[] when vdev is NULL (per Selva).
 - Update patch commit message.
 - Link to v1: https://lore.kernel.org/all/TYUPR06MB6217000B59003EDF233D7246D2B22@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

 drivers/usb/host/xhci-mem.c      |  9 +++++++++
 drivers/usb/host/xhci-sideband.c | 28 +++++++++++++++++++++-------
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index af8d4b74c4ba..448d28aaff3e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -15,6 +15,7 @@
 #include <linux/dmapool.h>
 #include <linux/dma-mapping.h>
 #include <linux/bitfield.h>
+#include <linux/usb/xhci-sideband.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -922,6 +923,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;
+		dev->sideband = NULL;
+	}
+
 	kfree(dev);
 }
 
diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index a5deeee4d5dc..f979ce517163 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -472,12 +472,25 @@ 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]);
+		} else {
+			for (i = 0; i < EP_CTX_PER_DEV; i++)
+				sb->eps[i] = NULL;
+		}
 
 		__xhci_sideband_remove_interrupter(sb);
 
@@ -486,7 +499,8 @@ xhci_sideband_unregister(struct xhci_sideband *sb)
 
 	spin_lock_irq(&xhci->lock);
 	sb->xhci = NULL;
-	vdev->sideband = NULL;
+	if (vdev)
+		vdev->sideband = NULL;
 	spin_unlock_irq(&xhci->lock);
 
 	kfree(sb);
-- 
2.48.1