USB: serial: fix port tear down use-after-free

PATCH New Johan Hovold 1 participant 5 messages Aug 21

Subsystems (3)

Stable branch USB USB serial

lore

Share & tools

lore

Johan Hovold

author maintainer

2026-08-21 15:46 UTC #0 lore raw

v2 cover USB: serial: fix port tear down use-after-free

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

Changes in v2:
- fix dynamic id driver deregistration race (new)
- remove "new_id" attributes before deregistering USB driver
- use iterator for driver deregistration (new)

Johan Hovold (4):
USB: serial: fix port tear down use-after-free
USB: serial: fix dynamic id driver deregistration race
USB: serial: fix driver deregistration order
USB: serial: use iterator for driver deregistration

 drivers/usb/serial/bus.c        |  6 +++++-
 drivers/usb/serial/usb-serial.c | 32 ++++++++++++++++++++++++++------
 include/linux/usb/serial.h      |  1 +
 3 files changed, 32 insertions(+), 7 deletions(-)

Signature · 1 line

2.54.0

Johan Hovold

author maintainer

2026-08-21 15:46 UTC #1 lore raw

PATCH v2 1/4 USB: serial: fix port tear down use-after-free

Some drivers for multiport devices access port driver data from completion handlers of shared URBs submitted at attach() or first open() and stopped at disconnect() or last close(), 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 [email protected]

Link https://lore.kernel.org/all/[email protected]/ open here

Cc [email protected] # 2.6.30

Cc Alan Stern <[email protected]>

Reviewed-by Greg Kroah-Hartman <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/usb-serial.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/usb/serial/usb-serial.c +7 -2

index 17edc057a311..a4fbc849c0fa 100644
@@ -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 */

Signature · 1 line

2.54.0

Johan Hovold

author maintainer

2026-08-21 15:46 UTC #2 lore raw

PATCH v2 2/4 USB: serial: fix dynamic id driver deregistration race

In reply to Johan Hovold

Only free the dynamic ids after having deregistered the driver and removed the "new_id" attribute to avoid leaking any id added by a racing write to the attribute.

Fixes 93bacefc4cc0 ("USB serial: add dynamic id support to usb-serial core")

Cc [email protected] # 2.6.21

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/serial/bus.c +1 -1

index 9e2a18c0b218..295c61afa44c 100644
@@ -165,7 +165,7 @@ int usb_serial_bus_register(struct usb_serial_driver *driver)
 
 void usb_serial_bus_deregister(struct usb_serial_driver *driver)
 {
-	free_dynids(driver);
 	driver_unregister(&driver->driver);
+	free_dynids(driver);
 }

Signature · 1 line

2.54.0

Johan Hovold

author maintainer

2026-08-21 15:46 UTC #3 lore raw

PATCH v2 3/4 USB: serial: fix driver deregistration order

In reply to 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 "new_id" attributes must first be removed to prevent new ids from being added and triggering a probe of the USB driver after it has been deregistered.

Fixes 765e0ba62613 ("usb-serial: new API for driver registration")

Cc [email protected] # 3.4

Cc Alan Stern <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/bus.c        |  4 ++++
 drivers/usb/serial/usb-serial.c | 21 ++++++++++++++++++---
 include/linux/usb/serial.h      |  1 +
 3 files changed, 23 insertions(+), 3 deletions(-)

drivers/usb/serial/bus.c +4 -0

index 295c61afa44c..ea1fe5d1e449 100644
@@ -169,3 +169,7 @@ void usb_serial_bus_deregister(struct usb_serial_driver *driver)
 	free_dynids(driver);
 }
 
+void usb_serial_bus_remove_new_id(struct usb_serial_driver *driver)
+{
+	driver_remove_file(&driver->driver, &driver_attr_new_id);
+}

drivers/usb/serial/usb-serial.c +18 -3

index a4fbc849c0fa..11a0ed6d6546 100644
@@ -1469,7 +1469,7 @@ int __usb_serial_register_drivers(struct usb_serial_driver *const serial_drivers
 {
 	int rc;
 	struct usb_driver *udriver;
-	struct usb_serial_driver * const *sd;
+	struct usb_serial_driver * const *sd, * const *s;
 
 	/*
 	 * udriver must be registered before any of the serial drivers,

@@ -1522,9 +1522,11 @@ int __usb_serial_register_drivers(struct usb_serial_driver *const serial_drivers
 	return 0;
 
 err_deregister_drivers:
+	for (s = serial_drivers; s < sd; ++s)
+		usb_serial_bus_remove_new_id(*s);
+	usb_deregister(udriver);
 	while (sd-- > serial_drivers)
 		usb_serial_deregister(*sd);
-	usb_deregister(udriver);
 err_free_driver:
 	kfree(udriver);
 	return rc;

@@ -1542,10 +1544,23 @@ EXPORT_SYMBOL_GPL(__usb_serial_register_drivers);
 void usb_serial_deregister_drivers(struct usb_serial_driver *const serial_drivers[])
 {
 	struct usb_driver *udriver = (*serial_drivers)->usb_driver;
+	struct usb_serial_driver * const *sd;
+
+	/*
+	 * udriver must be deregistered before the serial drivers so that
+	 * I/O is stopped before unbinding the ports.
+	 *
+	 * Remove the new_id attributes to prevent ids from being added and
+	 * triggering a probe of udriver after it has been deregistered.
+	 */
+	for (sd = serial_drivers; *sd; ++sd)
+		usb_serial_bus_remove_new_id(*sd);
+
+	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);

include/linux/usb/serial.h +1 -0

index 534e6650e2aa..ab0a32a90629 100644
@@ -381,6 +381,7 @@ void usb_serial_handle_dcd_change(struct usb_serial_port *usb_port,
 
 int usb_serial_bus_register(struct usb_serial_driver *device);
 void usb_serial_bus_deregister(struct usb_serial_driver *device);
+void usb_serial_bus_remove_new_id(struct usb_serial_driver *driver);
 
 extern const struct bus_type usb_serial_bus_type;
 extern struct tty_driver *usb_serial_tty_driver;

Signature · 1 line

2.54.0

Johan Hovold

author maintainer

2026-08-21 15:46 UTC #4 lore raw

PATCH v2 4/4 USB: serial: use iterator for driver deregistration

In reply to Johan Hovold

Use the new serial driver iterator variable also when deregistering serial drivers to improve readability.

Signed-off-by Johan Hovold <[email protected]>

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

drivers/usb/serial/usb-serial.c +2 -2

index 11a0ed6d6546..9c51b44e2284 100644
@@ -1558,8 +1558,8 @@ void usb_serial_deregister_drivers(struct usb_serial_driver *const serial_driver
 
 	usb_deregister(udriver);
 
-	for (; *serial_drivers; ++serial_drivers)
-		usb_serial_deregister(*serial_drivers);
+	for (sd = serial_drivers; *sd; ++sd)
+		usb_serial_deregister(*sd);
 
 	kfree(udriver);
 }

Signature · 1 line

2.54.0