usb: usbip: fix OOB read/write in usbip_pad_iso()

usbip_pad_iso() repositions ISO frame data within the transfer buffer
via memmove().  Neither the source offset (actualoffset, derived by
subtracting wire-supplied actual_length values) nor the destination
offset (iso_frame_desc[i].offset, taken directly from the wire) is
bounds-checked.

If a crafted actual_length wraps actualoffset negative through the
subtraction (see patch 2/3 for the root cause), the memmove source
points before the allocation - slab OOB read, data returned to
userspace.

Independently, iso_frame_desc[i].offset is never validated against
transfer_buffer_length.  Setting offset past the end of the buffer
gives a fully controlled OOB write into whatever sits next in the
slab - confirmed with offset=400 on a 392-byte buffer, 64-byte write.

Add bounds checks for both the source and destination ranges before
each memmove call.  Use unsigned comparisons after the sign check on
actualoffset to avoid signed/unsigned conversion surprises.

Signed-off-by: Kelvin Mbogo <addcontent08@gmail.com>
Link: https://patch.msgid.link/20260325103640.8090-3-addcontent08@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index fd620e96003947a2b8c35a7d5e70067f5f1e9880..8ebaaeaf848e5be5f93a4a79ea713cbe64f2bb4d 100644 (file)
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -770,6 +770,42 @@ void usbip_pad_iso(struct usbip_device *ud, struct urb *urb)
         */
        for (i = np-1; i > 0; i--) {
                actualoffset -= urb->iso_frame_desc[i].actual_length;
+
+               /*
+                * Validate source range: actualoffset can go negative
+                * via crafted actual_length values from the wire.
+                */
+               if (actualoffset < 0 ||
+                   (unsigned int)actualoffset >
+                               (unsigned int)urb->transfer_buffer_length ||
+                   urb->iso_frame_desc[i].actual_length >
+                               (unsigned int)urb->transfer_buffer_length -
+                               (unsigned int)actualoffset) {
+                       dev_err(&urb->dev->dev,
+                               "pad_iso: bad src off=%d len=%u bufsz=%d\n",
+                               actualoffset,
+                               urb->iso_frame_desc[i].actual_length,
+                               urb->transfer_buffer_length);
+                       return;
+               }
+
+               /*
+                * Validate destination range: iso_frame_desc[i].offset
+                * is wire-supplied and must not exceed the buffer.
+                */
+               if (urb->iso_frame_desc[i].offset >
+                               (unsigned int)urb->transfer_buffer_length ||
+                   urb->iso_frame_desc[i].actual_length >
+                               (unsigned int)urb->transfer_buffer_length -
+                               urb->iso_frame_desc[i].offset) {
+                       dev_err(&urb->dev->dev,
+                               "pad_iso: bad dst off=%u len=%u bufsz=%d\n",
+                               urb->iso_frame_desc[i].offset,
+                               urb->iso_frame_desc[i].actual_length,
+                               urb->transfer_buffer_length);
+                       return;
+               }
+
                memmove(urb->transfer_buffer + urb->iso_frame_desc[i].offset,
                        urb->transfer_buffer + actualoffset,
                        urb->iso_frame_desc[i].actual_length);