serial core: fix use-after-free after driver module unload

PATCH Superseded Johan Hovold 2 participants 3 messages Nov 12

Subsystems (2)

Stable branch TTY layer and serial drivers

lore

Share & tools

lore

Johan Hovold

author

2026-11-12 10:07 UTC #0 lore raw

PATCH 1/1 serial core: fix use-after-free after driver module unload

Deregistering a uart driver frees the driver uart state including the tty ports themselves.

Take a reference to the uart driver module at install() so that the uart state and resources like the tty driver name do not go away while a tty is still open.

This specifically avoids use-after-free when accessing the tty port and crashes when accessing the tty driver name (e.g. in the tty printk macros) when a tty is closed after its driver module has been unloaded.

It also makes sure that the uart state is still valid when waking up any waiters after releasing the final uart port reference in uart_port_deref().

Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")

Cc [email protected]

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

---

I hit this when investigating a deregistration race that can lead to a deadlock:

https://lore.kernel.org/lkml/[email protected]/

Johan

 drivers/tty/serial/serial_core.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

drivers/tty/serial/serial_core.c +17 -1

index 2e53d194ecfe..289f767cc068 100644
@@ -1930,10 +1930,25 @@ static int uart_install(struct tty_driver *driver, struct tty_struct *tty)
 {
 	struct uart_driver *drv = driver->driver_state;
 	struct uart_state *state = drv->state + tty->index;
+	int ret;
+
+	if (!try_module_get(drv->owner))
+		return -ENODEV;
 
 	tty->driver_data = state;
 
-	return tty_standard_install(driver, tty);
+	ret = tty_standard_install(driver, tty);
+	if (ret)
+		module_put(drv->owner);
+
+	return ret;
+}
+
+static void uart_cleanup(struct tty_struct *tty)
+{
+	struct uart_driver *drv = tty->driver->driver_state;
+
+	module_put(drv->owner);
 }
 
 /*

@@ -2733,6 +2748,7 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
 
 static const struct tty_operations uart_ops = {
 	.install	= uart_install,
+	.cleanup	= uart_cleanup,
 	.open		= uart_open,
 	.close		= uart_close,
 	.write		= uart_write,

Signature · 1 line

2.56.1

Johan Hovold

author

2026-11-12 10:12 UTC #1 lore raw

Quoted 2 lines · Johan Hovold

Deregistering a uart driver frees the driver uart state including the tty ports themselves.

I apparently forgot the colon after "serial" in Subject. Perhaps it can be added when the patch is applied.

Johan

[email protected]

AI review

2026-11-12 10:28 UTC #2 lore raw

In reply to Johan Hovold

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] serial: core: use-after-free and early module_put() in uart_cleanup() - [Low] serial: core…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] serial: core: use-after-free and early module_put() in uart_cleanup()
- [Low] serial: core: inaccurate Fixes trailer in commit message --

commit 6a2bc2d8089d9c95546b231a0390b02e8dc6cd97
Author: Johan Hovold <[email protected]>

Quoted 13 lines

serial core: fix use-after-free after driver module unload
Deregistering a uart driver frees the driver uart state including the tty ports themselves.
Take a reference to the uart driver module at install() so that the uart state and resources like the tty driver name do not go away while a tty is still open.
This specifically avoids use-after-free when accessing the tty port and crashes when accessing the tty driver name (e.g. in the tty printk macros) when a tty is closed after its driver module has been unloaded.

Quoted 7 lines

It also makes sure that the uart state is still valid when waking up any waiters after releasing the final uart port reference in uart_port_deref().
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc [email protected]
Signed-off-by Johan Hovold <[email protected]>

[Severity: Low]
1da177e4c3f4 ("Linux-2.6.12-rc2") cannot be resolved in the git history, and uart_register_driver() originally set normal->owner = drv->owner until it was removed by 2f16669d322e ("TTY: remove re-assignments to tty_driver members"). Should the Fixes tag reference that commit instead?

Quoted 19 lines

drivers/tty/serial/serial_core.c +7 -1

index b06d2649be343..2160e72780b45 100644
@@ -1935,10 +1935,25 @@ static int uart_install(struct tty_driver *driver, struct tty_struct *tty)
 {
 	struct uart_driver *drv = driver->driver_state;
 	struct uart_state *state = drv->state + tty->index;
+	int ret;
+
+	if (!try_module_get(drv->owner))
+		return -ENODEV;
 
 	tty->driver_data = state;
 
-	return tty_standard_install(driver, tty);
+	ret = tty_standard_install(driver, tty);
+	if (ret)
+		module_put(drv->owner);

drivers/tty/serial/serial_core.c static int uart_install(struct tty_driver *driver, struct tty_struct *tty) · line 1957

Quoted 10 lines

drivers/tty/serial/serial_core.c +9 -0

+
+	return ret;
+}
+
+static void uart_cleanup(struct tty_struct *tty)
+{
+	struct uart_driver *drv = tty->driver->driver_state;
+
+	module_put(drv->owner);
 }

[Severity: High]
Can uart_cleanup() dereference freed memory via tty->driver->driver_state when a device is unbound while a TTY is open?

Drivers such as sccnxp and tegra-tcu embed struct uart_driver inside devm-allocated device state and call uart_unregister_driver() from their .remove() callback. Holding a reference to drv->owner does not prevent a sysfs unbind, which unregisters the driver and frees the struct containing struct uart_driver while the TTY remains open:

CPU0 CPU1
sccnxp_remove()
uart_unregister_driver(&s->uart)
devres_release_all()
kfree(s)

                                      release_one_tty()
                                        uart_cleanup()
                                          drv = tty->driver->driver_state;
                                          module_put(drv->owner);  <-- use-after-free

In addition, release_one_tty() runs tty->ops->cleanup(tty) before tty_driver_kref_put(driver):

drivers/tty/tty_io.c:release_one_tty() {
	struct tty_driver *driver = tty->driver;
	struct module *owner = driver->owner;

	if (tty->ops->cleanup)
		tty->ops->cleanup(tty);

	tty_driver_kref_put(driver);
	module_put(owner);
}

Dropping drv->owner in uart_cleanup() allows a concurrent rmmod to finish uart_unregister_driver() and unmap the module before release_one_tty() calls tty_driver_kref_put(driver) -> destruct_tty_driver(), leaving /proc/tty/driver/<driver_name> registered while drv and drv->state are already freed. Leaving normal->owner unset also leaves cdev->owner unset, so an open() racing with module unload is not blocked in chrdev_open() and can call tty_driver_kref_put() in tty_open_by_driver() after the module has been unloaded.

Setting normal->owner = drv->owner (or passing drv->owner to __tty_alloc_driver()) in uart_register_driver() lets the TTY core manage the module reference via struct tty_driver without dereferencing tty->driver->driver_state in cleanup.

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1