Skip to content

Commit 2ab833a

Browse files
nathancrebellogregkh
authored andcommitted
usbip: validate number_of_packets in usbip_pack_ret_submit()
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 c6688ef ("usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input") and b78d830 ("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: 1325f85 ("staging: usbip: bugfix add number of packets for isochronous frames") Cc: stable <stable@kernel.org> Acked-by: Shuah Khan <skhan@linuxfoundation.org> Signed-off-by: Nathan Rebello <nathan.c.rebello@gmail.com> Link: https://patch.msgid.link/20260402085259.234-1-nathan.c.rebello@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent f880aac commit 2ab833a

1 file changed

Lines changed: 12 additions & 0 deletions

File tree

drivers/usb/usbip/usbip_common.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,18 @@ static void usbip_pack_ret_submit(struct usbip_header *pdu, struct urb *urb,
470470
urb->status = rpdu->status;
471471
urb->actual_length = rpdu->actual_length;
472472
urb->start_frame = rpdu->start_frame;
473+
/*
474+
* The number_of_packets field determines the length of
475+
* iso_frame_desc[], which is a flexible array allocated
476+
* at URB creation time. A response must never claim more
477+
* packets than originally submitted; doing so would cause
478+
* an out-of-bounds write in usbip_recv_iso() and
479+
* usbip_pad_iso(). Clamp to zero on violation so both
480+
* functions safely return early.
481+
*/
482+
if (rpdu->number_of_packets < 0 ||
483+
rpdu->number_of_packets > urb->number_of_packets)
484+
rpdu->number_of_packets = 0;
473485
urb->number_of_packets = rpdu->number_of_packets;
474486
urb->error_count = rpdu->error_count;
475487
}

0 commit comments

Comments
 (0)