[PATCH] USB: serial: generic: recover from bulk-in endpoint stall

[PATCH] USB: serial: generic: recover from bulk-in endpoint stall

[Julian Oes]

A USB-serial device can go permanently silent when its bulk-in
endpoint gets stalled (-EPIPE).

In my case, disconnecting a CDC ACM device causes my USB hub to stall
the bulk-in endpoint of an FTDI adapter on a sibling port: the FTDI's
bulk-in URB completes with -EPIPE and no further data arrives.
Recovery requires userspace to close and reopen the tty.

usb_serial_generic_read_bulk_callback() treats -EPIPE as fatal and
abandons the URB. My assumption is that for a halted bulk endpoint,
recovery is CLEAR_FEATURE(ENDPOINT_HALT) followed by resubmission.

The fix follows the pattern used in cdc-acm: On -EPIPE, defer recovery
to the existing per-port work, which clears the endpoint halt and
resubmits the read URBs.

With this patch, data from the FTDI adapter keeps arriving after the
unrelated CDC ACM device is unplugged.

Fixes: fc11efe2800f ("USB: serial: continue to read on errors")
Signed-off-by: Julian Oes <julian@oes.ch>
Tested-by: Julian Oes <julian@oes.ch>

---

Reproducer hardware:
- CDC ACM device: PX4 Pixhawk 6X (NuttX, 3185:0035)
- FTDI device: SiK telemetry radio (FTDI FT231X, 0403:6015)
- Hub: j5create JUH377 7-port hub.
       https://info.j5create.com/products/juh377

Tested on Linux 7.1-rc3 on Linux Mint 22.3 x86_64.

Debugging and patch authoring were assisted by AI.

usbmon traces of broken and fixed behaviour available on request.
---
 drivers/usb/serial/generic.c    | 12 ++++++++----
 drivers/usb/serial/usb-serial.c | 13 +++++++++++++
 include/linux/usb/serial.h      |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 757f0a586ddb..7f58ea55507b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -371,6 +371,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 	struct usb_serial_port *port = urb->context;
 	unsigned char *data = urb->transfer_buffer;
 	bool stopped = false;
+	bool stalled = false;
 	int status = urb->status;
 	int i;
 
@@ -395,9 +396,10 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 		stopped = true;
 		break;
 	case -EPIPE:
-		dev_err(&port->dev, "%s - urb stopped: %d\n",
-							__func__, status);
-		stopped = true;
+		dev_err(&port->dev, "%s - urb stalled: %d\n",
+			__func__, status);
+		set_bit(USB_SERIAL_RX_STALLED, &port->flags);
+		stalled = true;
 		break;
 	default:
 		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
@@ -420,7 +422,9 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 	 */
 	smp_mb__after_atomic();
 
-	if (stopped)
+	if (stalled)
+		schedule_work(&port->work);
+	if (stopped || stalled)
 		return;
 
 	if (test_bit(USB_SERIAL_THROTTLED, &port->flags))
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 0e072fd87c3d..383ff1d2fc35 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -629,6 +629,19 @@ static void usb_serial_port_work(struct work_struct *work)
 	struct usb_serial_port *port =
 		container_of(work, struct usb_serial_port, work);
 
+	if (test_and_clear_bit(USB_SERIAL_RX_STALLED, &port->flags)) {
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
+			usb_kill_urb(port->read_urbs[i]);
+
+		usb_clear_halt(port->serial->dev,
+			       usb_rcvbulkpipe(port->serial->dev,
+					       port->bulk_in_endpointAddress));
+
+		usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+	}
+
 	tty_port_tty_wakeup(&port->port);
 }
 
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 75b2b763f1ba..64dadb2cb7ea 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -20,6 +20,7 @@
 /* USB serial flags */
 #define USB_SERIAL_WRITE_BUSY	0
 #define USB_SERIAL_THROTTLED	1
+#define USB_SERIAL_RX_STALLED	2
 
 /**
  * usb_serial_port: structure for the specific ports of a device.
-- 
2.43.0