[PATCH v2] usb: usbfs: clamp actual_length to transfer_buffer_length

[Cheng Yongkang]

usbfs uses urb->actual_length from the host controller driver without
checking it against the buffer it allocated. For a completed
non-isochronous IN URB, processcompl() and processcompl_compat() copy
that many bytes to userspace and report the same value, and
async_completed() passes it to snoop_urb_data() when usbfs_snoop is
enabled.

With USB/IP, the value comes from the remote peer. If the peer returns
actual_length=88 for a 24-byte control-IN transfer, usbip_recv_xbuff()
rejects the length, but VHCI still gives back the URB with -EPROTO and
actual_length=88. usbfs copies data from failed URBs too, since they
can carry valid partial data, so reaping the URB reads past the buffer:

  BUG: KASAN: slab-out-of-bounds in _copy_to_user
   copy_urb_data_to_user

With usbfs_snoop enabled, hex_dump_to_buffer() reads past the buffer
earlier, when the URB completes.

Clamp urb->actual_length in async_completed() while holding ps->lock,
before the completion is snooped or can be reaped. The copy, the
reported length and the snoop dump then stay within
transfer_buffer_length.

Tested on usb-linus e971f4f365ba with KASAN, using a synthetic USB/IP
peer that returns 88 bytes for a 24-byte control-IN transfer. Without
this change, reaping the URB hits the report above, and with
usbfs_snoop enabled the completion hits it in hex_dump_to_buffer().
With it, the URB is reaped with -EPROTO and actual_length=24 in both
cases. Full and short control-IN and control-OUT transfers through
usbfs still work.

Fixes: 0880aef49e40a ("USB: usbfs_snoop: add data logging back in")
Fixes: d4a4683ca054e ("USB: usbfs: only copy the actual data received")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Cheng Yongkang <teel4res@xxxxxxxxx>
---
Changes in v2:
- Clamp urb->actual_length in async_completed() instead of only the
  copy length in copy_urb_data_to_user(), so the reported length and
  the snoop dump are bounded too, as pointed out in review [2].
- Fix the Fixes tags; v1 named a later scatter-gather refactor.
- Send separately. The USB/IP patch in v1 duplicates Sung Byeongchan's
  earlier submission [1], which covers only the VHCI receive-failure
  path, so usbfs still needs its own bound.

v1: https://lore.kernel.org/linux-usb/179328314074.648427.8187049654053191605@xxxxxxxxx/
[1]: https://lore.kernel.org/linux-usb/20261009122636.642575-1-tjdqudcks0424@xxxxxxxxx/
[2]: https://lore.kernel.org/linux-usb/sashiko-outbox-189547@xxxxxxxxxx/

 drivers/usb/core/devio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 101cb9425..7319e8b98 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -633,6 +633,8 @@ static void async_completed(struct urb *urb)
 	spin_lock_irqsave(&ps->lock, flags);
 	list_move_tail(&as->asynclist, &ps->async_completed);
 	as->status = urb->status;
+	if (urb->actual_length > urb->transfer_buffer_length)
+		urb->actual_length = urb->transfer_buffer_length;
 	signr = as->signr;
 	if (signr) {
 		errno = as->status;

base-commit: e971f4f365bad51d01bc90db55b56fdb3124f963
-- 
2.34.1