Re: [PATCH] usb: usbip: stub_rx: remove outdated comment

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usbip: protocol desync and hung URBs on invalid endpoint
--

commit f15a0a944a703fedd99ceab9d659df84a495556c
Author: Oliver Neukum <oneukum@suse.com>
Subject: usb: usbip: stub_rx: remove outdated comment

This patch removes an outdated comment in the error handling path of
get_pipe(). The comment indicated the code was unreachable, but following the
removal of a BUG() statement, this path can now be reached and logs a report.

> diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
> index 1e9ae578810d9..0ccc05b76c5ab 100644
> --- a/drivers/usb/usbip/stub_rx.c
> +++ b/drivers/usb/usbip/stub_rx.c
> @@ -391,7 +391,6 @@ static int get_pipe(struct stub_device *sdev, struct usbip_header *pdu)
>  	}
>  
>  err_ret:
> -	/* NOT REACHED */
>  	dev_err(&sdev->udev->dev, "CMD_SUBMIT: invalid epnum %d\n", epnum);
>  	return -1;

[Severity: High]
When get_pipe() returns -1, does it cause a protocol desync regression in
stub_recv_cmd_submit()?

If a USBIP client sends a CMD_SUBMIT with an invalid endpoint number or an
out-of-bounds number of isoc packets, get_pipe() returns -1.

Looking at drivers/usb/usbip/stub_rx.c:stub_recv_cmd_submit():

	int pipe = get_pipe(sdev, pdu);
	...
	if (pipe == -1)
		return;

When get_pipe() returns -1, stub_recv_cmd_submit() returns early without
consuming the TCP payload or issuing a connection teardown (e.g., via
usbip_event_add(ud, SDEV_EVENT_ERROR_TCP)).

For OUT transfers with a payload, the unread bytes remain in the socket
buffer. The next loop iteration reads this leftover payload as a USBIP
header, desynchronizing the protocol stream and abruptly terminating the
connection.

For IN transfers, no payload is left, but the server drops the request
without sending a RET_SUBMIT response.

Does this cause the client's URB to hang indefinitely?

If the client later unlinks this hung URB, it seems the server replies
with a RET_UNLINK status of 0 (success) because it never tracked the
dropped URB.

Could this trick the client into completing the failed URB with a false
success status, breaking the protocol state machine?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005125842.197848-1-oneukum@suse.com?part=1