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

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

[Nathan Rebello]

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

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

[Shuah Khan]

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>

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

[Greg KH]

On Tue, Mar 31, 2026 at 05:17:44PM -0600, Shuah Khan wrote:
> 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>

This patch is somehow corrupted and can not be applied at all.  Nathan,
how did you generate it?

You can tell something is wrong as it shows you removing, and then
adding, the same line, which is a huge hint something went wrong.

Can you regenerate this against my latest usb-testing branch and resend?

thanks,

greg k-h

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

[Nathan Rebello]

On Wed, Apr 02, 2026, Greg Kroah-Hartman wrote:
> This patch is somehow corrupted and can not be applied at all.  Nathan,
> how did you generate it?

I used git format-patch and git send-email — the patch had a
whitespace issue, sorry about that.

> Can you regenerate this against my latest usb-testing branch and resend?

Done — sent the v2 against your latest usb-testing.

Thanks,
Nathan Rebello