[PATCH 6/6] usbip: vhci_hcd: reject URBs from stale usb_device after port re-attach
flat view
COOLING10d
From: Raul E Rangel <hidden>
Date: 2026-10-21 15:08:18
Also in:
lkml, stable
Subsystem:
the rest, usb over ip driver, usb subsystem · Maintainers:
Linus Torvalds, Valentina Manea, Shuah Khan, Shuah Khan, Greg Kroah-Hartman
After vhci_shutdown_connection() runs, event_handler() immediately calls vhci_device_reset(), setting vdev->ud.status to VDEV_ST_NULL and clearing vdev->udev before usb_hub_wq finishes running usb_disconnect() on the detached usb_device. If attach_store() attaches a new connection to the same rhport before usb_hub_wq disconnects the old usb_device, vdev->ud.tcp_socket becomes non-NULL and vdev->ud.status becomes VDEV_ST_NOTASSIGNED. Because vhci_urb_enqueue() maps urb->dev->portnum to vdev without verifying that urb->dev matches vdev->udev, URBs submitted on the old usb_device pass the connection state checks and get sent to the new remote device (or, if a concurrent USBDEVFS_RESET resets the old usb_device to address 0, overwrite vdev->udev). Serialize vdev->udev clearing in vhci_device_reset() under vhci->lock and ud->lock (dropping old usb_device and socket references outside the spinlocks in both vhci_device_reset() and vhci_urb_enqueue()), and reject URBs in vhci_urb_enqueue() when urb->dev != vdev->udev and either usb_pipedevice(urb->pipe) != 0 or urb->dev is already registered (device_is_registered(&urb->dev->dev)). During initial enumeration and PORT_INIT_TRIES retries in hub_port_connect(), each candidate usb_device is unregistered until usb_new_device() runs after hub_port_init() has assigned vdev->udev, whereas an already-registered usb_device resetting via usb_reset_device() is only valid when urb->dev == vdev->udev. Fixes: 04679b3489e0 ("Staging: USB/IP: add client driver") Cc: <redacted> Assisted-by: Gemini Signed-off-by: Raul E Rangel <redacted> --- drivers/usb/usbip/vhci_hcd.c | 44 +++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 27dba51392b7..e8ff63c2276f 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c@@ -693,6 +693,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd); struct vhci *vhci = vhci_hcd->vhci; struct device *dev = &urb->dev->dev; + struct usb_device *old = NULL; u8 portnum = urb->dev->portnum; int ret = 0; struct vhci_device *vdev;
@@ -737,6 +738,17 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag kfree(priv); return -ENODEV; } + if ((usb_pipedevice(urb->pipe) != 0 || + device_is_registered(&urb->dev->dev)) && + urb->dev != vdev->udev) { + dev_err_ratelimited(dev, + "enqueue from stale usb_device on port %d\n", + vdev->rhport); + spin_unlock(&vdev->ud.lock); + spin_unlock_irqrestore(&vhci->lock, flags); + kfree(priv); + return -ENODEV; + } spin_unlock(&vdev->ud.lock); ret = usb_hcd_link_urb_to_ep(hcd, urb);
@@ -753,7 +765,6 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag * */ if (usb_pipedevice(urb->pipe) == 0) { - struct usb_device *old; __u8 type = usb_pipetype(urb->pipe); struct usb_ctrlrequest *ctrlreq = (struct usb_ctrlrequest *) urb->setup_packet;
@@ -764,13 +775,13 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag goto no_need_xmit; } - old = vdev->udev; switch (ctrlreq->bRequest) { case USB_REQ_SET_ADDRESS: /* set_address may come when a device is reset */ dev_info(dev, "SetAddress Request (%d) to port %d\n", ctrlreq->wValue, vdev->rhport); + old = vdev->udev; vdev->udev = usb_get_dev(urb->dev); /* * NOTE: A similar operation has been done via
@@ -783,7 +794,6 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag * should be dropped. */ dev_pm_syscore_device(&vdev->udev->dev, true); - usb_put_dev(old); spin_lock(&vdev->ud.lock); vdev->ud.status = VDEV_ST_USED;
@@ -802,6 +812,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag usbip_dbg_vhci_hc( "Not yet?:Get_Descriptor to device 0 (get max pipe size)\n"); + old = vdev->udev; vdev->udev = usb_get_dev(urb->dev); /* * Set syscore PM flag for the virtually attached
@@ -814,7 +825,6 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag * on disconnect. */ dev_pm_syscore_device(&vdev->udev->dev, true); - usb_put_dev(old); goto out; default:
@@ -832,6 +842,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag out: vhci_tx_urb(urb, vdev, priv); spin_unlock_irqrestore(&vhci->lock, flags); + usb_put_dev(old); return 0;
@@ -848,6 +859,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag spin_lock(&vhci->lock); } spin_unlock_irqrestore(&vhci->lock, flags); + usb_put_dev(old); return ret; }
@@ -1156,25 +1168,31 @@ static void vhci_shutdown_connection(struct usbip_device *ud) static void vhci_device_reset(struct usbip_device *ud) { struct vhci_device *vdev = container_of(ud, struct vhci_device, ud); - struct usb_device *old = vdev->udev; + struct vhci *vhci = vdev_to_vhci_hcd(vdev)->vhci; + struct usb_device *old; + struct socket *socket; unsigned long flags; - spin_lock_irqsave(&ud->lock, flags); + spin_lock_irqsave(&vhci->lock, flags); + spin_lock(&ud->lock); vdev->speed = 0; vdev->devid = 0; + old = vdev->udev; vdev->udev = NULL; - usb_put_dev(old); - if (ud->tcp_socket) { - sockfd_put(ud->tcp_socket); - ud->tcp_socket = NULL; - ud->sockfd = -1; - } + socket = ud->tcp_socket; + ud->tcp_socket = NULL; + ud->sockfd = -1; ud->status = VDEV_ST_NULL; - spin_unlock_irqrestore(&ud->lock, flags); + spin_unlock(&ud->lock); + spin_unlock_irqrestore(&vhci->lock, flags); + + usb_put_dev(old); + if (socket) + sockfd_put(socket); } static void vhci_device_unusable(struct usbip_device *ud)
--
2.56.0.212.g129ede20d9-goog