Re: [PATCH] usbip: fix number_of_packets corruption for non-isochronous transfers

["gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>]

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 <markus.mikonsaari@gofore.com>

How was this found and tested?

And did you forget an Assisted-by: tag?

> ---
>  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