Re: [PATCH] usbip: vhci: reject RET_SUBMIT with inflated number_of_packets

[Shuah Khan <skhan@linuxfoundation.org>]

On 3/27/26 00:44, Nathan Rebello wrote:
> When a USB/IP client receives a RET_SUBMIT response,
> usbip_pack_ret_submit() unconditionally overwrites
> urb->number_of_packets from the network PDU. This value is
> subsequently used as the loop bound in usbip_recv_iso() and
> usbip_pad_iso() to iterate over urb->iso_frame_desc[], a flexible
> array whose size was fixed at URB allocation time based on the
> *original* number_of_packets from the CMD_SUBMIT.
> 
> A malicious USB/IP server can set number_of_packets in the response
> to a value larger than what was originally submitted, causing a heap
> out-of-bounds write when usbip_recv_iso() writes to
> urb->iso_frame_desc[i] beyond the allocated region.
> 
> KASAN confirmed this with kernel 7.0.0-rc5:
> 
>    BUG: KASAN: slab-out-of-bounds in usbip_recv_iso+0x46a/0x640
>    Write of size 4 at addr ffff888106351d40 by task vhci_rx/69
> 
>    The buggy address is located 0 bytes to the right of
>     allocated 320-byte region [ffff888106351c00, ffff888106351d40)
> 
> The server side (stub_rx.c) and gadget side (vudc_rx.c) already
> validate number_of_packets in the CMD_SUBMIT path since commits
> c6688ef9f297 ("usbip: fix stub_rx: harden CMD_SUBMIT path to handle
> malicious input") and b78d830f0049 ("usbip: fix vudc_rx: harden
> CMD_SUBMIT path to handle malicious input"). The server side validates
> against USBIP_MAX_ISO_PACKETS because no URB exists yet at that point.
> On the client side we have the original URB, so we can use the tighter
> bound: the response must not exceed the original number_of_packets.
> 
> This mirrors the existing validation of actual_length against
> transfer_buffer_length in usbip_recv_xbuff(), which checks the
> response value against the original allocation size.
> 
> Kelvin Mbogo's series ("usb: usbip: fix integer overflow in
> usbip_recv_iso()", v2) hardens the receive-side functions themselves;
> this patch complements that work by catching the bad value at its
> source -- in usbip_pack_ret_submit() before the overwrite -- and
> using the tighter per-URB allocation bound rather than the global
> USBIP_MAX_ISO_PACKETS limit.
> 
> Fix this by checking rpdu->number_of_packets against
> urb->number_of_packets in usbip_pack_ret_submit() before the
> overwrite. On violation, clamp to zero so that usbip_recv_iso() and
> usbip_pad_iso() safely return early.
> 
> Fixes: 0775a9cbc798 ("staging: usbip: vhci extension: modifications to the client side")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nathan Rebello <nathan.c.rebello@gmail.com>
> ---
>   drivers/usb/usbip/usbip_common.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
> --- a/drivers/usb/usbip/usbip_common.c
> +++ b/drivers/usb/usbip/usbip_common.c
> @@ -470,7 +470,18 @@ static void usbip_pack_ret_submit(struct usbip_header *pdu, struct urb *urb,
>   		urb->status		= rpdu->status;
>   		urb->actual_length	= rpdu->actual_length;
>   		urb->start_frame	= rpdu->start_frame;
> -		urb->number_of_packets = rpdu->number_of_packets;
> +		/*
> +		 * The number_of_packets field determines the length of
> +		 * iso_frame_desc[], which is a flexible array allocated
> +		 * at URB creation time. A response must never claim more
> +		 * packets than originally submitted; doing so would cause
> +		 * an out-of-bounds write in usbip_recv_iso() and
> +		 * usbip_pad_iso(). Clamp to zero on violation so both
> +		 * functions safely return early.
> +		 */
> +		if (rpdu->number_of_packets < 0 ||
> +		    rpdu->number_of_packets > urb->number_of_packets)
> +			rpdu->number_of_packets = 0;
> +		urb->number_of_packets = rpdu->number_of_packets;
>   		urb->error_count	= rpdu->error_count;
>   	}
>   }

Look good to me.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>