[PATCH] usbip: Don't submit special requests twice

From: Simon Holesch
Date: Sun Nov 26 2023 - 18:54:19 EST


Even though submitting those requests twice should be harmless, there
are USB devices that react poorly to some duplicated requests.

One example is the ChipIdea controller implementation in U-Boot: The
second SET_CONFIURATION request makes U-Boot disable and re-enable all
endpoints. Re-enabling an endpoint in the ChipIdea controller, however,
was broken until U-Boot commit b272c8792502 ("usb: ci: Fix gadget
reinit").

Signed-off-by: Simon Holesch <simon@xxxxxxxxxx>
---
drivers/usb/usbip/stub_rx.c | 50 ++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index fc01b31bbb87..f3fcc290ce5b 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -167,13 +167,13 @@ static int tweak_reset_device_cmd(struct urb *urb)
/*
* clear_halt, set_interface, and set_configuration require special tricks.
*/
-static void tweak_special_requests(struct urb *urb)
+static int tweak_special_requests(struct urb *urb)
{
if (!urb || !urb->setup_packet)
- return;
+ return 0;

if (usb_pipetype(urb->pipe) != PIPE_CONTROL)
- return;
+ return 0;

if (is_clear_halt_cmd(urb))
/* tweak clear_halt */
@@ -189,8 +189,12 @@ static void tweak_special_requests(struct urb *urb)

else if (is_reset_device_cmd(urb))
tweak_reset_device_cmd(urb);
- else
+ else {
usbip_dbg_stub_rx("no need to tweak\n");
+ return 0;
+ }
+
+ return 1;
}

/*
@@ -468,6 +472,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
int support_sg = 1;
int np = 0;
int ret, i;
+ int is_tweaked;

if (pipe == -1)
return;
@@ -580,8 +585,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
priv->urbs[i]->pipe = pipe;
priv->urbs[i]->complete = stub_complete;

- /* no need to submit an intercepted request, but harmless? */
- tweak_special_requests(priv->urbs[i]);
+ is_tweaked = tweak_special_requests(priv->urbs[i]);

masking_bogus_flags(priv->urbs[i]);
}
@@ -594,22 +598,32 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,

/* urb is now ready to submit */
for (i = 0; i < priv->num_urbs; i++) {
- ret = usb_submit_urb(priv->urbs[i], GFP_KERNEL);
+ if (!is_tweaked) {
+ ret = usb_submit_urb(priv->urbs[i], GFP_KERNEL);

- if (ret == 0)
- usbip_dbg_stub_rx("submit urb ok, seqnum %u\n",
- pdu->base.seqnum);
- else {
- dev_err(&udev->dev, "submit_urb error, %d\n", ret);
- usbip_dump_header(pdu);
- usbip_dump_urb(priv->urbs[i]);
+ if (ret == 0)
+ usbip_dbg_stub_rx("submit urb ok, seqnum %u\n",
+ pdu->base.seqnum);
+ else {
+ dev_err(&udev->dev, "submit_urb error, %d\n", ret);
+ usbip_dump_header(pdu);
+ usbip_dump_urb(priv->urbs[i]);

+ /*
+ * Pessimistic.
+ * This connection will be discarded.
+ */
+ usbip_event_add(ud, SDEV_EVENT_ERROR_SUBMIT);
+ break;
+ }
+ } else {
/*
- * Pessimistic.
- * This connection will be discarded.
+ * An identical URB was already submitted in
+ * tweak_special_requests(). Skip submitting this URB to not
+ * duplicate the request.
*/
- usbip_event_add(ud, SDEV_EVENT_ERROR_SUBMIT);
- break;
+ priv->urbs[i]->status = 0;
+ stub_complete(priv->urbs[i]);
}
}

--
2.43.0