[PATCH] usbip: vhci: do not give back URBs with an unchecked actual_length

[PATCH] usbip: vhci: do not give back URBs with an unchecked actual_length

[Vlatko Kosturjak]

vhci_recv_ret_submit() copies actual_length from the RET_SUBMIT header
into the URB before usbip_recv_xbuff() checks it. When the server reports
more data than the URB's transfer buffer holds, usbip_recv_xbuff() rejects
the reply and shuts the connection down, but the URB is still given back
with the oversized actual_length (and status -EPROTO).

Completion handlers trust actual_length. usbfs copies that many bytes from
the transfer buffer to user space in copy_urb_data_to_user(), whatever the
URB status, so the kernel reads past the kmalloc()ed buffer. With
CONFIG_HARDENED_USERCOPY this is a BUG in the context of the process
reaping the URB:

  usb 1-1: recv xbuf, 0
  usercopy: Kernel memory exposure attempt detected from SLUB object 'kmalloc-rnd-05-16' (offset 0, size 22)!
  kernel BUG at mm/usercopy.c:102!
  RIP: 0010:usercopy_abort+0x6c/0x80
  Call Trace:
   copy_urb_data_to_user+0x122/0x160
   processcompl+0xe3/0x160
   usbdev_do_ioctl+0x14d/0x1900

Without it, the adjacent heap contents are copied to user space.

This was hit with a buggy user-space USB/IP device implementation (it
answered an IN URB with more data than requested) while pcscd read from
the device; any server, buggy or malicious, can trigger it.

Do not trust actual_length when the transfer buffer could not be
received: give the URB back with actual_length 0. A short read leaves the
buffer partly unfilled too, so this covers both failure cases.

Fixes: b348d7dddb6c ("USB: usbip: fix potential out-of-bounds write")
Cc: stable@vger.kernel.org
Signed-off-by: Vlatko Kosturjak <kost@linux.hr>
---
Tested on host 7.0 (Ubuntu 7.0.0-34-generic) in QEMU with a small reproducer: a
user-space program that attaches a fake device to vhci_hcd through a Unix
socketpair (no network or usbip tools needed), answers a 16-byte bulk IN
URB with actual_length 22, and reaps the URB through usbfs:

  unpatched, CONFIG_HARDENED_USERCOPY=y:  the BUG above
  unpatched, hardened_usercopy=off:       reaped actual_length 22 for a 16-byte
                                          buffer; with 200 extra bytes, 200 bytes
                                          of adjacent heap reach user space
  patched:                                reaped status -EPROTO, actual_length 0

The vhci_rx.c hunk is the same in v7.0 and in mainline. Happy to send the
reproducer (about 300 lines of C plus a QEMU script) if useful.

The 2026 hardening of RET_SUBMIT (number_of_packets, iso frame lengths)
does not cover this path; it is the non-isochronous actual_length.

 drivers/usb/usbip/vhci_rx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index a678e7c89..720dafa72 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -78,6 +78,14 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 
 	/* recv transfer buffer */
 	if (usbip_recv_xbuff(ud, urb) < 0) {
+		/*
+		 * The transfer buffer was not filled: either a short read or
+		 * an actual_length larger than transfer_buffer_length, which
+		 * usbip_recv_xbuff() rejects. Do not give the URB back with
+		 * that actual_length: completion handlers trust it, e.g.
+		 * usbfs copies actual_length bytes out of transfer_buffer.
+		 */
+		urb->actual_length = 0;
 		urb->status = -EPROTO;
 		goto error;
 	}

base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d