Re: [PATCH] usbip: fix number_of_packets corruption for non-isochronous transfers
flat view
From: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Date: 2026-09-09 08:46:32
On Wed, Sep 09, 2026 at 08:09:48AM +0000, Markus Mikonsaari wrote:
In the USB/IP protocol, number_of_packets is set to 0xffffffff (-1) by sender when the transfer is not isochronous. usbip_pack_pdu() copies this wire value into urb->number_of_packets unconditionally. A host controller driver may compute the iso_frame_desc memory requirements directly from number_of_packets without independently validating it against the pipe type which produces an undersized allocation.
What driver does that?
On dwc_otg, this manifests as a slab-out-of-bounds write in dwc_otg_hcd_urb_alloc() during a USB/IP attach involving a non-isochronous transfer. Correct the number_of_packets to the value the urb was actually allocated for immediately after usbip_pack_pdu() overwrites it. Signed-off-by: Markus Mikonsaari <redacted>
How was this found and tested? And did you forget an Assisted-by: tag?
quoted hunk ↗ jump to hunk
--- drivers/usb/usbip/stub_rx.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c index 1e9ae578810d..baf511024da2 100644 --- a/drivers/usb/usbip/stub_rx.c +++ b/drivers/usb/usbip/stub_rx.c@@ -567,6 +567,17 @@ static void stub_recv_cmd_submit(struct stub_device *sdev, } usbip_pack_pdu(pdu, priv->urbs[0], USBIP_CMD_SUBMIT, 0); + /* + * number_of_packets is set to -1 by the sender when the transfer is + * not isochronous. + * usbip_pack_pdu() copies this wire value into urb->number_of_packets + * unconditionally, instead of using the correct value in np which was + * used to allocate the urb above. For a non-isochronous transfer this + * leaves number_of_packets at -1 which downstream consumers of this urb + * like host-controller drivers use to allocate iso_frame_desc storage. + * Restore it to what the urb was actually allocated for. + */ + priv->urbs[0]->number_of_packets = np; } else { for_each_sg(sgl, sg, nents, i) { priv->urbs[i] = usb_alloc_urb(0, GFP_KERNEL);@@ -579,6 +590,8 @@ static void stub_recv_cmd_submit(struct stub_device *sdev, usbip_pack_pdu(pdu, priv->urbs[i], USBIP_CMD_SUBMIT, 0); priv->urbs[i]->transfer_buffer = sg_virt(sg); priv->urbs[i]->transfer_buffer_length = sg->length; + /* see comment about number_of_packets above */
What comment? That's not the best way to do this... thanks, greg k-h