[PATCH 0/4] URB disconnect bug fixes

[PATCH 0/4] URB disconnect bug fixes

[Martin Kelly]

This patch series is to fix a common bug I found in mcba_usb that appears to
also exist in these drivers. Because I don't own the hardware, I cannot test the
changes, but I wanted to send the change so that others could do so.

The bug is that, when you disconnect the device, you get -EPIPE and -EPROTO
error codes (exactly which depends on the timing and the system) in the URB read
callback. The driver then resubmits the URB, which immediately fails because the
device is unplugged. On a slower system like a Raspberry Pi, this can cause a
kernel stall, while on a desktop, the disconnect code runs fast enough to
prevent a stall and it's easy not to notice the issue.

Fix this by not resubmitting an URB that receives an -EPIPE/-EPROTO code.

Martin Kelly (4):
  can: ems_usb: cancel urb on -EPIPE and -EPROTO
  can: esd_usb2: cancel urb on -EPIPE and -EPROTO
  can: kvaser_usb: cancel urb on -EPIPE and -EPROTO
  can: usb_8dev: cancel urb on -EPIPE and -EPROTO

 drivers/net/can/usb/ems_usb.c    | 2 ++
 drivers/net/can/usb/esd_usb2.c   | 2 ++
 drivers/net/can/usb/kvaser_usb.c | 2 ++
 drivers/net/can/usb/usb_8dev.c   | 2 ++
 4 files changed, 8 insertions(+)

--
2.11.0

[PATCH 1/4] can: ems_usb: cancel urb on -EPIPE and -EPROTO

[Martin Kelly]

In mcba_usb, we have observed that when you unplug the device, the driver will
endlessly resubmit failing URBs, which can cause CPU stalls. This issue
is fixed in mcba_usb by catching the codes seen on device disconnect
(-EPIPE and -EPROTO).

This driver also resubmits in the case of -EPIPE and -EPROTO, so fix it
in the same way.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/ems_usb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index b3d02759c226..b00358297424 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -288,6 +288,8 @@ static void ems_usb_read_interrupt_callback(struct urb *urb)
 
 	case -ECONNRESET: /* unlink */
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0

[PATCH 2/4] can: esd_usb2: cancel urb on -EPIPE and -EPROTO

[Martin Kelly]

In mcba_usb, we have observed that when you unplug the device, the driver will
endlessly resubmit failing URBs, which can cause CPU stalls. This issue
is fixed in mcba_usb by catching the codes seen on device disconnect
(-EPIPE and -EPROTO).

This driver also resubmits in the case of -EPIPE and -EPROTO, so fix it
in the same way.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/esd_usb2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 9fdb0f0bfa06..c6dcf93675c0 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -393,6 +393,8 @@ static void esd_usb2_read_bulk_callback(struct urb *urb)
 		break;
 
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0

[PATCH 3/4] can: kvaser_usb: cancel urb on -EPIPE and -EPROTO

[Martin Kelly]

In mcba_usb, we have observed that when you unplug the device, the driver will
endlessly resubmit failing URBs, which can cause CPU stalls. This issue
is fixed in mcba_usb by catching the codes seen on device disconnect
(-EPIPE and -EPROTO).

This driver also resubmits in the case of -EPIPE and -EPROTO, so fix it
in the same way.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/kvaser_usb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index f95945915d20..63587b8e6825 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1326,6 +1326,8 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
 	case 0:
 		break;
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 	default:
-- 
2.11.0

[PATCH 4/4] can: usb_8dev: cancel urb on -EPIPE and -EPROTO

[Martin Kelly]

In mcba_usb, we have observed that when you unplug the device, the driver will
endlessly resubmit failing URBs, which can cause CPU stalls. This issue
is fixed in mcba_usb by catching the codes seen on device disconnect
(-EPIPE and -EPROTO).

This driver also resubmits in the case of -EPIPE and -EPROTO, so fix it
in the same way.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/usb_8dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c
index d000cb62d6ae..27861c417c94 100644
--- a/drivers/net/can/usb/usb_8dev.c
+++ b/drivers/net/can/usb/usb_8dev.c
@@ -524,6 +524,8 @@ static void usb_8dev_read_bulk_callback(struct urb *urb)
 		break;
 
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0

Re: [PATCH 0/4] URB disconnect bug fixes

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1166 bytes --]

On 12/05/2017 08:15 PM, Martin Kelly wrote:
> This patch series is to fix a common bug I found in mcba_usb that appears to
> also exist in these drivers. Because I don't own the hardware, I cannot test the
> changes, but I wanted to send the change so that others could do so.
> 
> The bug is that, when you disconnect the device, you get -EPIPE and -EPROTO
> error codes (exactly which depends on the timing and the system) in the URB read
> callback. The driver then resubmits the URB, which immediately fails because the
> device is unplugged. On a slower system like a Raspberry Pi, this can cause a
> kernel stall, while on a desktop, the disconnect code runs fast enough to
> prevent a stall and it's easy not to notice the issue.
> 
> Fix this by not resubmitting an URB that receives an -EPIPE/-EPROTO code.

Applied this series and the mcba patch.

Thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]