usb: usbip: fix use-after-free race

stub_recv_cmd_submit() allocates a priv structure, which is freed by the TX thread after all URBs in the priv complete and are handled. This means that stub_recv_cmd_submit() effectively loses ownership of priv once the final URB is submitted: in the worst case, the RX thread will be preempted before usb_submit_urb() returns, and the TX thread will do all handling and cleanup before the RX thread resumes.

We don't lose ownership if usb_submit_urb() returns an error value, since that means it won't eventually call stub_complete(), so the error-handling usbip_dump_urb(priv->urbs[i]) is still safe.

---
 drivers/usb/usbip/stub_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index fc01b31bbb87..dba9a64830e6 100644
@@ -592,8 +592,11 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
 	if (usbip_recv_iso(ud, priv->urbs[0]) < 0)
 		return;
 
-	/* urb is now ready to submit */
-	for (i = 0; i < priv->num_urbs; i++) {
+	/*
+	 * URB(s) are now ready to submit.
+	 * Note: priv is freed after the last URB is (successfully) submitted.
+	 */
+	for (i = 0; i < num_urbs; i++) {
 		ret = usb_submit_urb(priv->urbs[i], GFP_KERNEL);
 
 		if (ret == 0)

2.41.0

How did you find this problem? Please add the details in the change log and also please describe the fix in detail.

This patch changes for loop from looping on priv->num_urbs to num_urbs. Change looks okay to me. I do want to know how you found the problem.

Did you happen to track down where this urb gets free'd after usbip_dump_urb(priv->urbs[i])