https://lore.kernel.org/linux-usb/20191206032406.GE1208@mail-itl/T/#u
In this mail thread, it shows system hang when there is receive
error in vhci. There are two different causes in this bug.
[1] Wrong receive logic in vhci when using scatter-gather
[2] Wrong error path of vhci_recv_ret_submit()
[1] considers normal reception to be an error condition and closes
connection. And when [1] error situation occurs, wrong error path[2]
causes the system freeze. So each patch fixes this bugs.
---
Change log
Patch [1] - Add Tested-by tag
Patch [2] - Add Tested-by tag
- Fix typo
- Fix error code in urb->status (-EPIPE->-EPROTO)
Suwan Kim (2):
usbip: Fix receive error in vhci-hcd when using scatter-gather
usbip: Fix error path of vhci_recv_ret_submit()
drivers/usb/usbip/usbip_common.c | 3 +++
drivers/usb/usbip/vhci_rx.c | 13 +++++++++----
2 files changed, 12 insertions(+), 4 deletions(-)
--
2.20.1
When vhci uses SG and receives data whose size is smaller than SG
buffer size, it tries to receive more data even if it acutally
receives all the data from the server. If then, it erroneously adds
error event and triggers connection shutdown.
vhci-hcd should check if it received all the data even if there are
more SG entries left. So, check if it receivces all the data from
the server in for_each_sg() loop.
Fixes: ea44d190764b ("usbip: Implement SG support to vhci-hcd and stub driver")
Reported-by: Marek Marczykowski-Górecki <[email protected]>
Tested-by: Marek Marczykowski-Górecki <[email protected]>
Signed-off-by: Suwan Kim <[email protected]>
---
drivers/usb/usbip/usbip_common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index 6532d68e8808..e4b96674c405 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -727,6 +727,9 @@ int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb)
copy -= recv;
ret += recv;
+
+ if (!copy)
+ break;
}
if (ret != size)
--
2.20.1
If a transaction error happens in vhci_recv_ret_submit(), event
handler closes connection and changes port status to kick hub_event.
Then hub tries to flush the endpoint URBs, but that causes infinite
loop between usb_hub_flush_endpoint() and vhci_urb_dequeue() because
"vhci_priv" in vhci_urb_dequeue() was already released by
vhci_recv_ret_submit() before a transmission error occurred. Thus,
vhci_urb_dequeue() terminates early and usb_hub_flush_endpoint()
continuously calls vhci_urb_dequeue().
The root cause of this issue is that vhci_recv_ret_submit()
terminates early without giving back URB when transaction error
occurs in vhci_recv_ret_submit(). That causes the error URB to still
be linked at endpoint list without “vhci_priv".
So, in the case of transaction error in vhci_recv_ret_submit(),
unlink URB from the endpoint, insert proper error code in
urb->status and give back URB.
Reported-by: Marek Marczykowski-Górecki <[email protected]>
Tested-by: Marek Marczykowski-Górecki <[email protected]>
Signed-off-by: Suwan Kim <[email protected]>
---
drivers/usb/usbip/vhci_rx.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index 33f8972ba842..00fc98741c5d 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -77,16 +77,21 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
usbip_pack_pdu(pdu, urb, USBIP_RET_SUBMIT, 0);
/* recv transfer buffer */
- if (usbip_recv_xbuff(ud, urb) < 0)
- return;
+ if (usbip_recv_xbuff(ud, urb) < 0) {
+ urb->status = -EPROTO;
+ goto error;
+ }
/* recv iso_packet_descriptor */
- if (usbip_recv_iso(ud, urb) < 0)
- return;
+ if (usbip_recv_iso(ud, urb) < 0) {
+ urb->status = -EPROTO;
+ goto error;
+ }
/* restore the padding in iso packets */
usbip_pad_iso(ud, urb);
+error:
if (usbip_dbg_flag_vhci_rx)
usbip_dump_urb(urb);
--
2.20.1