[PATCH 6/6] usbip: vhci_hcd: reject URBs from stale usb_device after port re-attach

[Raul E Rangel <rrangel@chromium.org>]

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: stable@vger.kernel.org
Assisted-by: Gemini
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
---

 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