[PATCH] USB: serial: oti6858: fix the teardown order in close

[PATCH] USB: serial: oti6858: fix the teardown order in close

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

The driver passes a baton around: the completion handler of the
interrupt urb either resubmits the urb, submits the read urb or
schedules one of the two delayed works, and the works and the bulk
completion handlers submit the interrupt urb again.

oti6858_close() cancels the works first and kills the urbs afterwards.
An interrupt urb that completes in between schedules a work again:

	oti6858_close()
	  cancel_delayed_work_sync(setup)
	  cancel_delayed_work_sync(write)
				oti6858_read_int_callback()
				  schedule_delayed_work(setup)
	  usb_kill_urb(interrupt_in_urb)	/* not in flight */
				setup_line()
				  usb_submit_urb(interrupt_in_urb)

The port is now closed with the baton still going round. The next open
fails because the interrupt urb is already submitted, and if the device
is unplugged, oti6858_port_remove() frees the private data while a work
may be pending or running.

Poison the interrupt urb first: that waits for the handler and makes all
further submissions fail, so nothing schedules the works any more. Then
cancel the works, after which only the bulk urbs are left, kill those,
and make the interrupt urb usable again for the next open. Don't print
an error for the submissions that fail with -EPERM in this window, as
oti6858_read_bulk_callback() already does.

Fixes: 49cdee0ed0fc ("USB: oti6858 usb-serial driver (in Nokia CA-42 cable)")
Cc: stable <stable@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Johan, another llm-found patch, seems sane, but it's not exactly obvious
to me if this is real or not?

 drivers/usb/serial/oti6858.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/oti6858.c b/drivers/usb/serial/oti6858.c
index c6364e8ff71f..005fe19c3cd4 100644
--- a/drivers/usb/serial/oti6858.c
+++ b/drivers/usb/serial/oti6858.c
@@ -248,7 +248,7 @@ static void setup_line(struct work_struct *work)
 
 	dev_dbg(&port->dev, "%s(): submitting interrupt urb\n", __func__);
 	result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
-	if (result != 0) {
+	if (result != 0 && result != -EPERM) {
 		dev_err(&port->dev, "%s(): usb_submit_urb() failed with error %d\n",
 			__func__, result);
 	}
@@ -300,7 +300,7 @@ static void send_data(struct work_struct *work)
 
 		dev_dbg(&port->dev, "%s(): submitting interrupt urb\n", __func__);
 		result = usb_submit_urb(port->interrupt_in_urb, GFP_NOIO);
-		if (result != 0) {
+		if (result != 0 && result != -EPERM) {
 			dev_err(&port->dev, "%s(): usb_submit_urb() failed with error %d\n",
 				__func__, result);
 		}
@@ -561,6 +561,13 @@ static void oti6858_close(struct usb_serial_port *port)
 
 	dev_dbg(&port->dev, "%s(): after buf_clear()\n", __func__);
 
+	/*
+	 * The interrupt urb schedules the works, and the works and the bulk
+	 * urbs resubmit the interrupt urb: keep it from being submitted until
+	 * all of them are gone.
+	 */
+	usb_poison_urb(port->interrupt_in_urb);
+
 	/* cancel scheduled setup */
 	cancel_delayed_work_sync(&priv->delayed_setup_work);
 	cancel_delayed_work_sync(&priv->delayed_write_work);
@@ -569,7 +576,8 @@ static void oti6858_close(struct usb_serial_port *port)
 	dev_dbg(&port->dev, "%s(): shutting down urbs\n", __func__);
 	usb_kill_urb(port->write_urb);
 	usb_kill_urb(port->read_urb);
-	usb_kill_urb(port->interrupt_in_urb);
+
+	usb_unpoison_urb(port->interrupt_in_urb);
 }
 
 static int oti6858_tiocmset(struct tty_struct *tty,
@@ -826,7 +834,7 @@ static void oti6858_write_bulk_callback(struct urb *urb)
 	/* schedule the interrupt urb if we are still open */
 	dev_dbg(&port->dev, "%s(): submitting interrupt urb\n", __func__);
 	result = usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC);
-	if (result != 0) {
+	if (result != 0 && result != -EPERM) {
 		dev_err(&port->dev, "%s(): failed submitting int urb,"
 					" error %d\n", __func__, result);
 	}
-- 
2.56.0

Re: [PATCH] USB: serial: oti6858: fix the teardown order in close

[sashiko-bot]

> From: Josef Bacik <josef@toxicpanda.com>
> 
> The driver passes a baton around: the completion handler of the
> interrupt urb either resubmits the urb, submits the read urb or
> schedules one of the two delayed works, and the works and the bulk
> completion handlers submit the interrupt urb again.
> 
> oti6858_close() cancels the works first and kills the urbs afterwards.
> An interrupt urb that completes in between schedules a work again:
> 
> 	oti6858_close()
> 	  cancel_delayed_work_sync(setup)
> 	  cancel_delayed_work_sync(write)
> 				oti6858_read_int_callback()
> 				  schedule_delayed_work(setup)
> [ ... ]
> Fixes: 49cdee0ed0fc ("USB: oti6858 usb-serial driver (in Nokia CA-42 cable)")
> Cc: stable <stable@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026102725-congenial-profound-99ee@gregkh?part=1

Re: [PATCH] USB: serial: oti6858: fix the teardown order in close

[Johan Hovold]

On Tue, Oct 27, 2026 at 03:31:26PM +0100, Greg Kroah-Hartman wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> The driver passes a baton around: the completion handler of the
> interrupt urb either resubmits the urb, submits the read urb or
> schedules one of the two delayed works, and the works and the bulk
> completion handlers submit the interrupt urb again.
> 
> oti6858_close() cancels the works first and kills the urbs afterwards.
> An interrupt urb that completes in between schedules a work again:
> 
> 	oti6858_close()
> 	  cancel_delayed_work_sync(setup)
> 	  cancel_delayed_work_sync(write)
> 				oti6858_read_int_callback()
> 				  schedule_delayed_work(setup)
> 	  usb_kill_urb(interrupt_in_urb)	/* not in flight */
> 				setup_line()
> 				  usb_submit_urb(interrupt_in_urb)
> 
> The port is now closed with the baton still going round. The next open
> fails because the interrupt urb is already submitted, and if the device
> is unplugged, oti6858_port_remove() frees the private data while a work
> may be pending or running.
>
> Poison the interrupt urb first: that waits for the handler and makes all
> further submissions fail, so nothing schedules the works any more. Then
> cancel the works, after which only the bulk urbs are left, kill those,
> and make the interrupt urb usable again for the next open. Don't print
> an error for the submissions that fail with -EPERM in this window, as
> oti6858_read_bulk_callback() already does.
> 
> Fixes: 49cdee0ed0fc ("USB: oti6858 usb-serial driver (in Nokia CA-42 cable)")
> Cc: stable <stable@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Johan, another llm-found patch, seems sane, but it's not exactly obvious
> to me if this is real or not?

Yes, this all looks good. I'll queue this one for 7.4. Thanks.

Johan