[PATCH 0/2] USB: serial: fix port tear down use-after-free

[PATCH 0/2] USB: serial: fix port tear down use-after-free

[Johan Hovold]

When addressing a port probe ordering issue in digi_acceleport recently
I realised that we have a corresponding general problem during
disconnect, but I didn't have time to address it straight away.

Now syzbot has managed to trigger one of its symptoms and people have
already started submitting incomplete band-aids so here is a proper
fix.

When testing the fix I also noticed that we have an ordering issue when
deregistering drivers that can result in similar use-after-free.

Johan


Johan Hovold (2):
  USB: serial: fix port tear down use-after-free
  USB: serial: fix driver deregistration order

 drivers/usb/serial/usb-serial.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

-- 
2.54.0

[PATCH 1/2] USB: serial: fix port tear down use-after-free

[Johan Hovold]

Some drivers for multiport devices access port driver data from
completion handlers of shared URBs submitted at attach() or first open()
and stopped at last close() or disconnect(), respectively.

A simple NULL check before accessing the driver data makes sure that a
port state container has at least been allocated, but a completion
handler can still race with port tear down.

Reorder the disconnect handling so that ports are not deregistered (and
their driver data freed) until after all ports have been hung up and the
driver disconnect() callback has run so that all I/O has been stopped.

Fixes: 2d93148ab698 ("USB: serial: fix lifetime and locking problems")
Reported-by: syzbot+e5e28c3e953b2eebb16e@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/6a7e6fb9.ec5dc6cc.21cb3f.00c0.GAE@google.com/
Cc: stable@vger.kernel.org	# 2.6.30
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/usb-serial.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 17edc057a311..a4fbc849c0fa 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1191,12 +1191,17 @@ static void usb_serial_disconnect(struct usb_interface *interface)
 		usb_serial_port_poison_urbs(port);
 		wake_up_interruptible(&port->port.delta_msr_wait);
 		cancel_work_sync(&port->work);
-		if (device_is_registered(&port->dev))
-			device_del(&port->dev);
 	}
+
 	if (serial->type->disconnect)
 		serial->type->disconnect(serial);
 
+	for (i = 0; i < serial->num_ports; ++i) {
+		port = serial->port[i];
+		if (device_is_registered(&port->dev))
+			device_del(&port->dev);
+	}
+
 	release_sibling(serial, interface);
 
 	/* let the last holder of this object cause it to be cleaned up */
-- 
2.54.0

[PATCH 2/2] USB: serial: fix driver deregistration order

[Johan Hovold]

USB serial driver modules register one driver for the USB bus and one or
more drivers for the ports on the USB serial bus.

When unloading a driver module, the USB driver must be deregistered
before the USB serial bus drivers so that I/O is stopped before
unbinding the ports to avoid use-after-free in completion handlers
accessing port data.

Note that the order does not matter currently in the registration error
path as the USB driver is not bound until after the USB serial drivers
have been registered.

Fixes: 765e0ba62613 ("usb-serial: new API for driver registration")
Cc: stable@vger.kernel.org	# 3.4
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/usb-serial.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index a4fbc849c0fa..08a9b6f8cf7d 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1522,9 +1522,9 @@ int __usb_serial_register_drivers(struct usb_serial_driver *const serial_drivers
 	return 0;
 
 err_deregister_drivers:
+	usb_deregister(udriver);
 	while (sd-- > serial_drivers)
 		usb_serial_deregister(*sd);
-	usb_deregister(udriver);
 err_free_driver:
 	kfree(udriver);
 	return rc;
@@ -1543,9 +1543,15 @@ void usb_serial_deregister_drivers(struct usb_serial_driver *const serial_driver
 {
 	struct usb_driver *udriver = (*serial_drivers)->usb_driver;
 
+	/*
+	 * The USB driver must be deregistered before the USB serial drivers
+	 * so that I/O is stopped before unbinding the ports.
+	 */
+	usb_deregister(udriver);
+
 	for (; *serial_drivers; ++serial_drivers)
 		usb_serial_deregister(*serial_drivers);
-	usb_deregister(udriver);
+
 	kfree(udriver);
 }
 EXPORT_SYMBOL_GPL(usb_serial_deregister_drivers);
-- 
2.54.0

Re: [PATCH 0/2] USB: serial: fix port tear down use-after-free

[Greg Kroah-Hartman]

On Thu, Aug 20, 2026 at 04:54:00PM +0200, Johan Hovold wrote:
> When addressing a port probe ordering issue in digi_acceleport recently
> I realised that we have a corresponding general problem during
> disconnect, but I didn't have time to address it straight away.
> 
> Now syzbot has managed to trigger one of its symptoms and people have
> already started submitting incomplete band-aids so here is a proper
> fix.
> 
> When testing the fix I also noticed that we have an ordering issue when
> deregistering drivers that can result in similar use-after-free.

Nice catch:


Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 2/2] USB: serial: fix driver deregistration order

[Alan Stern]

On Thu, Aug 20, 2026 at 04:54:02PM +0200, Johan Hovold wrote:
> USB serial driver modules register one driver for the USB bus and one or
> more drivers for the ports on the USB serial bus.
> 
> When unloading a driver module, the USB driver must be deregistered
> before the USB serial bus drivers so that I/O is stopped before
> unbinding the ports to avoid use-after-free in completion handlers
> accessing port data.
> 
> Note that the order does not matter currently in the registration error
> path as the USB driver is not bound until after the USB serial drivers
> have been registered.

But __usb_serial_register_drivers() does usb_register(udriver) _before_ 
calling usb_serial_register() for the serial_drivers.  Not _after_, as 
claimed here.

And if the order in the registration error path does not matter, why 
does the patch change it?

Or did you mean something else?

Also, what about the failure mode described in 765e0ba62613?  Not to 
mention that it seems odd to register a parent device driver after a 
child device driver, rather than before.  Or to unregister them in the 
reverse order.

If the point is to avoid I/O operations completing after the device 
structure has been deallocated, why not rely on the serial-bus drivers 
to stop all their I/O when they are unbound?

If these questions don't make sense, remember that I haven't done 
serious work on this subsystem for 15 years or so and blame it on that.  
:-)

Alan Stern

> Fixes: 765e0ba62613 ("usb-serial: new API for driver registration")
> Cc: stable@vger.kernel.org	# 3.4
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/usb-serial.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index a4fbc849c0fa..08a9b6f8cf7d 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -1522,9 +1522,9 @@ int __usb_serial_register_drivers(struct usb_serial_driver *const serial_drivers
>  	return 0;
>  
>  err_deregister_drivers:
> +	usb_deregister(udriver);
>  	while (sd-- > serial_drivers)
>  		usb_serial_deregister(*sd);
> -	usb_deregister(udriver);
>  err_free_driver:
>  	kfree(udriver);
>  	return rc;
> @@ -1543,9 +1543,15 @@ void usb_serial_deregister_drivers(struct usb_serial_driver *const serial_driver
>  {
>  	struct usb_driver *udriver = (*serial_drivers)->usb_driver;
>  
> +	/*
> +	 * The USB driver must be deregistered before the USB serial drivers
> +	 * so that I/O is stopped before unbinding the ports.
> +	 */
> +	usb_deregister(udriver);
> +
>  	for (; *serial_drivers; ++serial_drivers)
>  		usb_serial_deregister(*serial_drivers);
> -	usb_deregister(udriver);
> +
>  	kfree(udriver);
>  }
>  EXPORT_SYMBOL_GPL(usb_serial_deregister_drivers);
> -- 
> 2.54.0
>