[PATCH] usbip: prevent the use of unavailable urb and hcpriv in tx

[Edward Adam Davis]

A race condition between TX and RX causes the URB to enter an unstable
state, triggering [1].

Similarly, a race condition exists between TX and URB reaping, leading
to [2].

In vhci_send_cmd_submit(), the URB refer count is temporarily incremented
to prevent RX or the reaping process from prematurely freeing a URB that
has not yet finished transmitting.

The priv_lock is used to protect access to hcpriv, preventing it from being
prematurely freed by RX while TX is unaware.

[1]
BUG: KASAN: slab-use-after-free in vhci_send_cmd_submit+0xed2/0x10c0 drivers/usb/usbip/vhci_tx.c:91
Read of size 4 at addr ffff888022a66584 by task vhci_tx/6044
Call Trace:
 vhci_send_cmd_submit+0xed2/0x10c0 drivers/usb/usbip/vhci_tx.c:91
 vhci_tx_loop+0xff/0x460 drivers/usb/usbip/vhci_tx.c:241
 
Allocated by task 6031:
 usb_alloc_urb+0x66/0xa0 drivers/usb/core/urb.c:75
 alloc_async drivers/usb/core/devio.c:408 [inline]
 proc_do_submiturb+0x741/0x3820 drivers/usb/core/devio.c:1780
 proc_submiturb drivers/usb/core/devio.c:2002 [inline]
 usbdev_do_ioctl drivers/usb/core/devio.c:2702 [inline]
 usbdev_ioctl+0x2adb/0x3aa0 drivers/usb/core/devio.c:2826

Freed by task 6031:
 urb_destroy drivers/usb/core/urb.c:27 [inline]
 kref_put include/linux/kref.h:65 [inline]
 usb_free_urb.part.0+0xa5/0x110 drivers/usb/core/urb.c:96
 usb_free_urb+0x1f/0x30 drivers/usb/core/urb.c:95
 free_async+0x358/0x530 drivers/usb/core/devio.c:435
 proc_reapurbnonblock drivers/usb/core/devio.c:2129 [inline]
 usbdev_do_ioctl drivers/usb/core/devio.c:2621 [inline]
 usbdev_ioctl+0x2e7/0x3aa0 drivers/usb/core/devio.c:2826

[2]
BUG: KASAN: slab-use-after-free in vhci_send_cmd_submit+0xe87/0x1110
Read of size 8 at addr ffff888020de4820 by task vhci_tx/9324
Call Trace:
vhci_send_cmd_submit drivers/usb/usbip/vhci_tx.c:158
vhci_tx_loop drivers/usb/usbip/vhci_tx.c:256

Allocated by task 9322:
vhci_urb_enqueue drivers/usb/usbip/vhci_hcd.c:674
usb_hcd_submit_urb drivers/usb/core/hcd.c:1542
usb_submit_urb drivers/usb/core/urb.c:586
proc_do_submiturb drivers/usb/core/devio.c:1968
usbdev_ioctl drivers/usb/core/devio.c:2003

Freed by task 9323:
pickup_urb_and_free_priv drivers/usb/usbip/vhci_rx.c:46
vhci_rx_loop drivers/usb/usbip/vhci_rx.c:66

Fixes: ddeee0b2eec2 ("USB: usbfs: properly clean up the as structure on error paths")
Fixes: 04679b3489e0 ("Staging: USB/IP: add client driver")
Reported-by: syzbot+ecbb1750082f7528b507@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=ecbb1750082f7528b507
Tested-by: syzbot+ecbb1750082f7528b507@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
---
 drivers/usb/usbip/vhci_tx.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/usbip/vhci_tx.c b/drivers/usb/usbip/vhci_tx.c
index 32e6fabccf72..03fc070d5825 100644
--- a/drivers/usb/usbip/vhci_tx.c
+++ b/drivers/usb/usbip/vhci_tx.c
@@ -68,7 +68,9 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 		int ret;
 		struct urb *urb = priv->urb;
 		struct usbip_header pdu_header;
+		unsigned long flags;
 
+		usb_get_urb(urb);
 		txsize = 0;
 		memset(&pdu_header, 0, sizeof(pdu_header));
 		memset(&msg, 0, sizeof(msg));
@@ -85,6 +87,7 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 		iov = kzalloc_objs(*iov, iovnum);
 		if (!iov) {
 			usbip_event_add(&vdev->ud, SDEV_EVENT_ERROR_MALLOC);
+			usb_put_urb(urb);
 			return -ENOMEM;
 		}
 
@@ -92,7 +95,15 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 			urb->transfer_flags |= URB_DMA_MAP_SG;
 
 		/* 1. setup usbip_header */
+		spin_lock_irqsave(&vdev->priv_lock, flags);
+		if (!urb->hcpriv) {
+			err = -EIO;
+			spin_unlock_irqrestore(&vdev->priv_lock, flags);
+			usb_put_urb(urb);
+			goto err_iso_buffer;
+		}
 		setup_cmd_submit_pdu(&pdu_header, urb);
+		spin_unlock_irqrestore(&vdev->priv_lock, flags);
 		usbip_header_correct_endian(&pdu_header, 1);
 		iovnum = 0;
 
@@ -127,6 +138,7 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 			if (!iso_buffer) {
 				usbip_event_add(&vdev->ud,
 						SDEV_EVENT_ERROR_MALLOC);
+				usb_put_urb(urb);
 				goto err_iso_buffer;
 			}
 
@@ -143,8 +155,10 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev)
 			       txsize);
 			usbip_event_add(&vdev->ud, VDEV_EVENT_ERROR_TCP);
 			err = -EPIPE;
+			usb_put_urb(urb);
 			goto err_tx;
 		}
+		usb_put_urb(urb);
 
 		kfree(iov);
 		/* This is only for isochronous case */
-- 
2.43.0