usbip: Don't submit special requests twice

Skip submitting URBs, when identical requests were already sent in
tweak_special_requests(). Instead call the completion handler directly
to return the result of the URB.

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_CONFIGURATION 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@holesch.de>
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Reviewed-by: Hongren Zheng <i@zenithal.me>
Tested-by: Hongren Zheng <i@zenithal.me>
Link: https://lore.kernel.org/r/20240519141922.171460-1-simon@holesch.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Simon Holesch and committed by

Greg Kroah-Hartman 2 years ago 8b6b386f 804da867

+50 -27

1 changed file

expand all

drivers

usb

usbip

stub_rx.c

+50 -27

drivers/usb/usbip/stub_rx.c

··· 144 144 if (err && err != -ENODEV) 145 145 dev_err(&sdev->udev->dev, "can't set config #%d, error %d\n", 146 146 config, err); 147 - return 0; 147 + return err; 148 148 } 149 149 150 150 static int tweak_reset_device_cmd(struct urb *urb) 151 151 { 152 152 struct stub_priv *priv = (struct stub_priv *) urb->context; 153 153 struct stub_device *sdev = priv->sdev; 154 + int err; 154 155 155 156 dev_info(&urb->dev->dev, "usb_queue_reset_device\n"); 156 157 157 - if (usb_lock_device_for_reset(sdev->udev, NULL) < 0) { 158 + err = usb_lock_device_for_reset(sdev->udev, NULL); 159 + if (err < 0) { 158 160 dev_err(&urb->dev->dev, "could not obtain lock to reset device\n"); 159 - return 0; 161 + return err; 160 162 } 161 - usb_reset_device(sdev->udev); 163 + err = usb_reset_device(sdev->udev); 162 164 usb_unlock_device(sdev->udev); 163 165 164 - return 0; 166 + return err; 165 167 } 166 168 167 169 /* 168 170 * clear_halt, set_interface, and set_configuration require special tricks. 171 + * Returns 1 if request was tweaked, 0 otherwise. 169 172 */ 170 - static void tweak_special_requests(struct urb *urb) 173 + static int tweak_special_requests(struct urb *urb) 171 174 { 175 + int err; 176 + 172 177 if (!urb || !urb->setup_packet) 173 - return; 178 + return 0; 174 179 175 180 if (usb_pipetype(urb->pipe) != PIPE_CONTROL) 176 - return; 181 + return 0; 177 182 178 183 if (is_clear_halt_cmd(urb)) 179 184 /* tweak clear_halt */ 180 - tweak_clear_halt_cmd(urb); 185 + err = tweak_clear_halt_cmd(urb); 181 186 182 187 else if (is_set_interface_cmd(urb)) 183 188 /* tweak set_interface */ 184 - tweak_set_interface_cmd(urb); 189 + err = tweak_set_interface_cmd(urb); 185 190 186 191 else if (is_set_configuration_cmd(urb)) 187 192 /* tweak set_configuration */ 188 - tweak_set_configuration_cmd(urb); 193 + err = tweak_set_configuration_cmd(urb); 189 194 190 195 else if (is_reset_device_cmd(urb)) 191 - tweak_reset_device_cmd(urb); 192 - else 196 + err = tweak_reset_device_cmd(urb); 197 + else { 193 198 usbip_dbg_stub_rx("no need to tweak\n"); 199 + return 0; 200 + } 201 + 202 + return !err; 194 203 } 195 204 196 205 /* ··· 477 468 int support_sg = 1; 478 469 int np = 0; 479 470 int ret, i; 471 + int is_tweaked; 480 472 481 473 if (pipe == -1) 482 474 return; ··· 590 580 priv->urbs[i]->pipe = pipe; 591 581 priv->urbs[i]->complete = stub_complete; 592 582 593 - /* no need to submit an intercepted request, but harmless? */ 594 - tweak_special_requests(priv->urbs[i]); 583 + /* 584 + * all URBs belong to a single PDU, so a global is_tweaked flag is 585 + * enough 586 + */ 587 + is_tweaked = tweak_special_requests(priv->urbs[i]); 595 588 596 589 masking_bogus_flags(priv->urbs[i]); 597 590 } ··· 607 594 608 595 /* urb is now ready to submit */ 609 596 for (i = 0; i < priv->num_urbs; i++) { 610 - ret = usb_submit_urb(priv->urbs[i], GFP_KERNEL); 597 + if (!is_tweaked) { 598 + ret = usb_submit_urb(priv->urbs[i], GFP_KERNEL); 611 599 612 - if (ret == 0) 613 - usbip_dbg_stub_rx("submit urb ok, seqnum %u\n", 614 - pdu->base.seqnum); 615 - else { 616 - dev_err(&udev->dev, "submit_urb error, %d\n", ret); 617 - usbip_dump_header(pdu); 618 - usbip_dump_urb(priv->urbs[i]); 600 + if (ret == 0) 601 + usbip_dbg_stub_rx("submit urb ok, seqnum %u\n", 602 + pdu->base.seqnum); 603 + else { 604 + dev_err(&udev->dev, "submit_urb error, %d\n", ret); 605 + usbip_dump_header(pdu); 606 + usbip_dump_urb(priv->urbs[i]); 619 607 608 + /* 609 + * Pessimistic. 610 + * This connection will be discarded. 611 + */ 612 + usbip_event_add(ud, SDEV_EVENT_ERROR_SUBMIT); 613 + break; 614 + } 615 + } else { 620 616 /* 621 - * Pessimistic. 622 - * This connection will be discarded. 617 + * An identical URB was already submitted in 618 + * tweak_special_requests(). Skip submitting this URB to not 619 + * duplicate the request. 623 620 */ 624 - usbip_event_add(ud, SDEV_EVENT_ERROR_SUBMIT); 625 - break; 621 + priv->urbs[i]->status = 0; 622 + stub_complete(priv->urbs[i]); 626 623 } 627 624 } 628 625

Configure Feed

Configure Feed