[PATCH] usbip: avoid dereferencing a missing device in receive errors

[PATCH] usbip: avoid dereferencing a missing device in receive errors

[Sung Byeongchan]

VUDC builds synthetic URBs which do not have a struct usb_device.  A peer
which closes or truncates an OUT transfer after the submit header makes the
shared USB/IP receive error path log through urb->dev.  The resulting NULL
device dereference crashes the kernel before the protocol error can be
handled.

Guard every shared receive and ISO diagnostic which can consume a VUDC URB.
The protocol error and teardown still run, while ordinary URBs retain their
device-scoped diagnostics.

On ff47652a4b66, a real VUDC interrupt-OUT endpoint reproduced the KASAN
NULL dereference in three fresh boots.  Normal and near-negative transfers
passed three times each.  With this change, the identical short transfer
returned a bounded error, reconnected, and completed a following valid
transfer in three fresh boots; normal controls also passed.  The touched
source is byte-identical and the object builds successfully at
47324d3a5b3a.  This demonstrates availability loss only, not RCE or LPE.

This issue was found through manual source inspection assisted by an
AI/LLM, then evaluated with local production-path dynamic validation.

Fixes: 79c02cb1fd5c ("usbip: vudc: Add vudc_rx")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
---
 drivers/usb/usbip/usbip_common.c | 52 ++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index 100f8920624bb..8d379a9437efa 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -683,8 +683,9 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
 		return 0;
 
 	if (np <= 0 || np > USBIP_MAX_ISO_PACKETS) {
-		dev_err(&urb->dev->dev,
-			"recv iso: invalid number_of_packets %d\n", np);
+		if (urb->dev)
+			dev_err(&urb->dev->dev,
+				"recv iso: invalid number_of_packets %d\n", np);
 		/*
 		 * usbip_pack_ret_submit() already set urb->number_of_packets
 		 * from the wire.  Zero it so processcompl() does not iterate
@@ -702,8 +703,9 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
 
 	ret = usbip_recv(ud->tcp_socket, buff, size);
 	if (ret != size) {
-		dev_err(&urb->dev->dev, "recv iso_frame_descriptor, %d\n",
-			ret);
+		if (urb->dev)
+			dev_err(&urb->dev->dev,
+				"recv iso_frame_descriptor, %d\n", ret);
 		kfree(buff);
 
 		if (ud->side == USBIP_STUB || ud->side == USBIP_VUDC)
@@ -720,10 +722,11 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
 		usbip_pack_iso(&iso[i], &urb->iso_frame_desc[i], 0);
 		if (urb->iso_frame_desc[i].actual_length >
 				(unsigned int)urb->transfer_buffer_length) {
-			dev_err(&urb->dev->dev,
-				"recv iso: frame actual_length %u exceeds buffer %d\n",
-				urb->iso_frame_desc[i].actual_length,
-				urb->transfer_buffer_length);
+			if (urb->dev)
+				dev_err(&urb->dev->dev,
+					"recv iso: frame actual_length %u exceeds buffer %d\n",
+					urb->iso_frame_desc[i].actual_length,
+					urb->transfer_buffer_length);
 			kfree(buff);
 			return -EPROTO;
 		}
@@ -733,9 +736,10 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
 	kfree(buff);
 
 	if (total_length != (u32)urb->actual_length) {
-		dev_err(&urb->dev->dev,
-			"total length of iso packets %u not equal to actual length of buffer %d\n",
-			total_length, urb->actual_length);
+		if (urb->dev)
+			dev_err(&urb->dev->dev,
+				"total length of iso packets %u not equal to actual length of buffer %d\n",
+				total_length, urb->actual_length);
 
 		if (ud->side == USBIP_STUB || ud->side == USBIP_VUDC)
 			usbip_event_add(ud, SDEV_EVENT_ERROR_TCP);
@@ -793,11 +797,12 @@ void usbip_pad_iso(struct usbip_device *ud, struct urb *urb)
 		    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);
+			if (urb->dev)
+				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;
 		}
 
@@ -810,11 +815,12 @@ void usbip_pad_iso(struct usbip_device *ud, struct urb *urb)
 		    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);
+			if (urb->dev)
+				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;
 		}
 
@@ -891,7 +897,9 @@ int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb)
 	return ret;
 
 error:
-	dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret);
+	/* VUDC creates synthetic URBs without a struct usb_device. */
+	if (urb->dev)
+		dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret);
 	if (ud->side == USBIP_STUB || ud->side == USBIP_VUDC)
 		usbip_event_add(ud, SDEV_EVENT_ERROR_TCP);
 	else

base-commit: 47324d3a5b3abd781295044d01d92d09f184e872
-- 
2.43.0

Re: [PATCH] usbip: avoid dereferencing a missing device in receive errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usbip: integer signedness in usbip_recv_xbuff() allows OOB read
--

commit 02833e7622ef2723c4209138f2286c80d50e18e5
Author: Sung Byeongchan <tjdqudcks0424@naver.com>

usbip: avoid dereferencing a missing device in receive errors

This commit adds NULL checks for urb->dev across several shared receive and
ISO diagnostic paths in USB/IP. This prevents kernel crashes when a missing
device dereference occurs, such as when processing synthetic URBs built by
VUDC.

> diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
> index 100f8920624bb..8d379a9437efa 100644
> --- a/drivers/usb/usbip/usbip_common.c
> +++ b/drivers/usb/usbip/usbip_common.c

[ ... ]

> @@ -891,7 +897,9 @@ int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb)
>  	return ret;
>  
>  error:
> -	dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret);
> +	/* VUDC creates synthetic URBs without a struct usb_device. */
> +	if (urb->dev)
> +		dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret);

[Severity: High]
Does the signedness of the size variable earlier in usbip_recv_xbuff() allow
an out-of-bounds read?

When handling the actual length, urb->actual_length (which is a u32) is
implicitly converted to a signed int:

	} else {
		/* the direction of urb must be IN. */
		if (usb_pipeout(urb->pipe))
			return 0;

		size = urb->actual_length;
	}

	/* no need to recv xbuff */
	if (!(size > 0))
		return 0;

	if (size > urb->transfer_buffer_length)
		/* should not happen, probably malicious packet */
		goto error;

If a malicious USB/IP server sends a USBIP_RET_SUBMIT packet with an
actual_length greater than 0x7FFFFFFF, size will become negative.

The check !(size > 0) will then evaluate to true, causing the function to
immediately return 0 and bypass the bounds check against
urb->transfer_buffer_length that normally jumps to this error path.

Could this allow the client driver to process the buffer using the forged
actual_length, resulting in an out-of-bounds read beyond its allocated
transfer_buffer?

>  	if (ud->side == USBIP_STUB || ud->side == USBIP_VUDC)
>  		usbip_event_add(ud, SDEV_EVENT_ERROR_TCP);
>  	else

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008134107.265827-1-tjdqudcks0424@naver.com?part=1