usbip: vhci: validate ret_submit number_of_packets

vhci_recv_ret_submit() unpacks USBIP_RET_SUBMIT directly into the URB, including number_of_packets from the remote server. For isochronous URBs, iso_frame_desc[] was allocated using the original locally submitted number_of_packets.

If a malicious or buggy USB/IP server returns a larger number_of_packets, usbip_recv_iso() will iterate past the end of urb->iso_frame_desc[] and write attacker-controlled ISO descriptors out of bounds. Later completion paths may also walk past iso_frame_desc[] if the poisoned number_of_packets is left in the URB after rejecting the response.

Fix this by saving the original packet count before unpacking the PDU, rejecting larger values from the server, restoring the original count on error, and marking the connection as broken.

---
 drivers/usb/usbip/vhci_rx.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

index a75f4a898a41..5bbfd5ae7755 100644
@@ -60,6 +60,7 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 	struct usbip_device *ud = &vdev->ud;
 	struct urb *urb;
 	unsigned long flags;
+	int orig_number_of_packets;
 
 	spin_lock_irqsave(&vdev->priv_lock, flags);
 	urb = pickup_urb_and_free_priv(vdev, pdu->base.seqnum);

@@ -73,9 +74,33 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 		return;
 	}
 
+	/*
+	 * Save the original number_of_packets before it gets overwritten
+	 * by the server's response. The iso_frame_desc[] array was allocated
+	 * based on this value, so the server must not increase it.
+	 */
+	orig_number_of_packets = urb->number_of_packets;
+
 	/* unpack the pdu to a urb */
 	usbip_pack_pdu(pdu, urb, USBIP_RET_SUBMIT, 0);
 
+	/*
+	 * Validate number_of_packets from the server response against the
+	 * original URB allocation. A malicious server could set this to a
+	 * larger value, causing usbip_recv_iso() to write beyond the
+	 * iso_frame_desc[] array bounds.
+	 */
+	if (urb->number_of_packets < 0 ||
+	    urb->number_of_packets > orig_number_of_packets) {
+		dev_err(&urb->dev->dev,
+			"invalid number_of_packets in ret_submit: %d (max %d)\n",
+			urb->number_of_packets, orig_number_of_packets);
+		urb->number_of_packets = orig_number_of_packets;
+		urb->status = -EPROTO;
+		usbip_event_add(ud, VDEV_EVENT_ERROR_TCP);
+		goto error;
+	}
+
 	/* recv transfer buffer */
 	if (usbip_recv_xbuff(ud, urb) < 0) {
 		urb->status = -EPROTO;

2.51.0

We need a "real name" here please.

Also, this really looks like the same patch sent here:

	https://lore.kernel.org/r/[email protected]

Is everyone forgetting to disclose that they are using AI tools to generate these things?

thanks,

greg k-h

Thank you for your time and for pointing these out. I sincerely apologize for the oversight.

I cloned the repository a few days ago and discovered the bugs, but I must admit that I used AI tools to figure out how to patch, and generate the patch and description.

I was not aware of the earlier submission
https://lore.kernel.org/r/[email protected].
Since this issue has been correctly fixed by the other author, please ignore my submission.
I have realized my mistake, and i will correct it.
I sincerely apologize again. Really sorry for the trouble.

thanks,

ZhiTao Ou