Ksawlii/kernel_samsung_a53x
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

usbip: Don't submit special requests twice

Browse source 
[ Upstream commit 8b6b386f9aa936ed0c190446c71cf59d4a507690 ]

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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Simon Holesch 2024-05-19 16:15:38 +02:00 • committed by Ksawlii
parent c57db7a29a
commit 7bd88d4e69
1 changed files with 50 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
drivers/usb/usbip/stub_rx.c
View file

@ -144,53 +144,62 @@ static int tweak_set_configuration_cmd(struct urb *urb)
if (err && err != -ENODEV) if (err && err != -ENODEV)
dev_err(&sdev->udev->dev, "can't set config #%d, error %d\n", dev_err(&sdev->udev->dev, "can't set config #%d, error %d\n",
config, err); config, err);
return 0; return err;
} }
static int tweak_reset_device_cmd(struct urb *urb) static int tweak_reset_device_cmd(struct urb *urb)
{ {
struct stub_priv *priv = (struct stub_priv *) urb->context; struct stub_priv *priv = (struct stub_priv *) urb->context;
struct stub_device *sdev = priv->sdev; struct stub_device *sdev = priv->sdev;
int err;
dev_info(&urb->dev->dev, "usb_queue_reset_device\n"); dev_info(&urb->dev->dev, "usb_queue_reset_device\n");
if (usb_lock_device_for_reset(sdev->udev, NULL) < 0) { err = usb_lock_device_for_reset(sdev->udev, NULL);
if (err < 0) {
dev_err(&urb->dev->dev, "could not obtain lock to reset device\n"); dev_err(&urb->dev->dev, "could not obtain lock to reset device\n");
return 0; return err;
} }
usb_reset_device(sdev->udev); err = usb_reset_device(sdev->udev);
usb_unlock_device(sdev->udev); usb_unlock_device(sdev->udev);
return 0; return err;
} }
/* /*
* clear_halt, set_interface, and set_configuration require special tricks. * clear_halt, set_interface, and set_configuration require special tricks.
* Returns 1 if request was tweaked, 0 otherwise.
*/ */
static void tweak_special_requests(struct urb *urb) static int tweak_special_requests(struct urb *urb)
{ {
int err;
if (!urb || !urb->setup_packet) if (!urb || !urb->setup_packet)
return; return 0;
if (usb_pipetype(urb->pipe) != PIPE_CONTROL) if (usb_pipetype(urb->pipe) != PIPE_CONTROL)
return; return 0;
if (is_clear_halt_cmd(urb)) if (is_clear_halt_cmd(urb))
/* tweak clear_halt */ /* tweak clear_halt */
tweak_clear_halt_cmd(urb); err = tweak_clear_halt_cmd(urb);
else if (is_set_interface_cmd(urb)) else if (is_set_interface_cmd(urb))
/* tweak set_interface */ /* tweak set_interface */
tweak_set_interface_cmd(urb); err = tweak_set_interface_cmd(urb);
else if (is_set_configuration_cmd(urb)) else if (is_set_configuration_cmd(urb))
/* tweak set_configuration */ /* tweak set_configuration */
tweak_set_configuration_cmd(urb); err = tweak_set_configuration_cmd(urb);
else if (is_reset_device_cmd(urb)) else if (is_reset_device_cmd(urb))
tweak_reset_device_cmd(urb); err = tweak_reset_device_cmd(urb);
else else {
usbip_dbg_stub_rx("no need to tweak\n"); usbip_dbg_stub_rx("no need to tweak\n");
return 0;
}
return !err;
} }
/* /*
@ -468,6 +477,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
int support_sg = 1; int support_sg = 1;
int np = 0; int np = 0;
int ret, i; int ret, i;
int is_tweaked;
if (pipe == -1) if (pipe == -1)
return; return;
@ -580,8 +590,11 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
priv->urbs[i]->pipe = pipe; priv->urbs[i]->pipe = pipe;
priv->urbs[i]->complete = stub_complete; priv->urbs[i]->complete = stub_complete;
/* no need to submit an intercepted request, but harmless? */ /*
tweak_special_requests(priv->urbs[i]); * all URBs belong to a single PDU, so a global is_tweaked flag is
* enough
*/
is_tweaked = tweak_special_requests(priv->urbs[i]);
masking_bogus_flags(priv->urbs[i]); masking_bogus_flags(priv->urbs[i]);
} }
@ -594,22 +607,32 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
/* urb is now ready to submit */ /* urb is now ready to submit */
for (i = 0; i < priv->num_urbs; i++) { 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) if (ret == 0)
usbip_dbg_stub_rx("submit urb ok, seqnum %u\n", usbip_dbg_stub_rx("submit urb ok, seqnum %u\n",
pdu->base.seqnum); pdu->base.seqnum);
else { else {
dev_err(&udev->dev, "submit_urb error, %d\n", ret); dev_err(&udev->dev, "submit_urb error, %d\n", ret);
usbip_dump_header(pdu); usbip_dump_header(pdu);
usbip_dump_urb(priv->urbs[i]); usbip_dump_urb(priv->urbs[i]);
/*
* Pessimistic.
* This connection will be discarded.
*/
usbip_event_add(ud, SDEV_EVENT_ERROR_SUBMIT);
break;
}
} else {
/* /*
* Pessimistic. * An identical URB was already submitted in
* This connection will be discarded. * tweak_special_requests(). Skip submitting this URB to not
* duplicate the request.
*/ */
usbip_event_add(ud, SDEV_EVENT_ERROR_SUBMIT); priv->urbs[i]->status = 0;
break; stub_complete(priv->urbs[i]);
} }
} }