[PATCH v2 3/3] usb: misc: usbio: bound the debug hex dumps by the received length

["HE WEI (ギカク)" <skyexpoc@gmail.com>]

usbio_ctrl_msg() and usbio_bulk_msg() hex dump the reply with "%*phN",
using a length the device supplied and that has not been validated yet:

	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_IN, 0, 0,
			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
	dev_dbg(usbio->dev, "control in %d hdr %*phN data %*phN\n", ret,
		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);

cpkt->len is a u8 read back out of usbio->ctrlbuf after the IN transfer,
and bpkt_len in usbio_bulk_msg() is le16_to_cpu(bpkt->len) read out of
usbio->rxbuf.  Both are handed to "%*phN" as the field width.
hex_string() in lib/vsprintf.c caps that at 64, and dereferences every
byte up to that cap regardless of how much room the output buffer has:

	if (spec.field_width > 0)
		len = min(spec.field_width, 64);

	for (i = 0; i < len; ++i) {
		if (buf < end)
			*buf = hex_asc_hi(addr[i]);

So the dump reads up to byte 67 of ctrlbuf and byte 68 of rxbuf.  Both
buffers are sized from the endpoint packet sizes, so this is out of
bounds whenever ep0 wMaxPacketSize is below 68 and whenever the bulk in
endpoint is below 69.  That covers every low, full and high speed ep0
(8, 16, 32 or 64) and the bulk sizes the supported bridges actually use
(64, or 63 for the Synaptics Sabre via USBIO_QUIRK_BULK_MAXP_63).

A device answering one of the five usbio_ctrl_msg() calls in
usbio_probe() with cpkt->len = 255 therefore leaks up to 60 bytes of
adjacent slab memory into the kernel log during enumeration, before any
user space is involved.  On the bulk path a reply claiming
bpkt->len = 0xffff reads 5 bytes past a 64 byte rxbuf.

Unlike the endpoint size issues this needs no malformed descriptor at
all; it only needs the dev_dbg() calls to be enabled.  When they are, it
is a slab-out-of-bounds read under KASAN and the bytes reach dmesg.

Bound both dumps by the number of bytes actually received.  That is in
bounds by construction and also stops the dump printing stale bytes left
over from an earlier transfer.  The matching dumps on the two OUT paths
are left alone: they use lengths the driver itself just wrote, which the
size checks above them already bound.

Found by code review.  The out-of-bounds read was reproduced under
AddressSanitizer with a userspace model of the two dev_dbg() call sites
and of hex_string()'s field-width handling; it has not been exercised on
hardware or on dummy_hcd.

Fixes: 121a0f839dbb ("usb: misc: Add Intel USBIO bridge driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5 asan
Signed-off-by: HE WEI (ギカク) <skyexpoc@gmail.com>
---
--- a/drivers/usb/misc/usbio.c
+++ b/drivers/usb/misc/usbio.c
@@ -14,6 +14,7 @@
 #include <linux/dev_printk.h>
 #include <linux/device.h>
 #include <linux/lockdep.h>
+#include <linux/minmax.h>
 #include <linux/mutex.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -141,7 +142,7 @@
 	struct usbio_ctrl_packet *cpkt;
 	unsigned int pipe;
 	u16 cpkt_len;
-	int ret;
+	int dbg_len, ret;
 
 	lockdep_assert_held(&usbio->ctrl_mutex);
 
@@ -181,8 +182,15 @@
 	cpkt_len = sizeof(*cpkt) + ibuf_len;
 	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_IN, 0, 0,
 			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
+	/*
+	 * cpkt->len has just been written by the device and is not validated
+	 * until below, while %*phN dereferences up to 64 bytes of whatever
+	 * field width it is handed.  Bound the dump by what was received.
+	 */
+	dbg_len = (ret > (int)sizeof(*cpkt)) ?
+		  min_t(int, cpkt->len, ret - (int)sizeof(*cpkt)) : 0;
 	dev_dbg(usbio->dev, "control in %d hdr %*phN data %*phN\n", ret,
-		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);
+		(int)sizeof(*cpkt), cpkt, dbg_len, cpkt->data);
 
 	if (ret < sizeof(*cpkt)) {
 		dev_err(usbio->dev, "USB control in failed: %d\n", ret);
@@ -258,7 +266,7 @@
 	struct usbio_client *client = adev_to_client(adev);
 	struct usbio_device *usbio = client->bridge;
 	struct usbio_bulk_packet *bpkt;
-	int ret, act = 0;
+	int ret, act = 0, dbg_len;
 	u16 bpkt_len;
 
 	lockdep_assert_held(&client->mutex);
@@ -314,8 +322,14 @@
 	act = usbio->rxdat_len;
 	bpkt = usbio->rxbuf;
 	bpkt_len = le16_to_cpu(bpkt->len);
+	/*
+	 * Same as in usbio_ctrl_msg(): bpkt_len is device supplied and is only
+	 * validated below, so bound the dump by what was received.
+	 */
+	dbg_len = (act > (int)sizeof(*bpkt)) ?
+		  min_t(int, bpkt_len, act - (int)sizeof(*bpkt)) : 0;
 	dev_dbg(usbio->dev, "bulk in %d hdr %*phN data %*phN\n", act,
-		(int)sizeof(*bpkt), bpkt, bpkt_len, bpkt->data);
+		(int)sizeof(*bpkt), bpkt, dbg_len, bpkt->data);
 
 	/*
 	 * Unsupported bulk commands get only an usbio_packet_header with
-- 
2.51.0