[PATCH] serial core: fix use-after-free after driver module unload

[PATCH] serial core: fix use-after-free after driver module unload

[Johan Hovold]

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: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---

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

	https://lore.kernel.org/lkml/avSAyV6QE0Id3rAk@hovoldconsulting.com/

Johan


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

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2e53d194ecfe..289f767cc068 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -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,
-- 
2.56.1

Re: [PATCH] serial core: fix use-after-free after driver module unload

[Johan Hovold]

On Thu, Nov 12, 2026 at 11:05:41AM +0100, Johan Hovold wrote:
> 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

Re: [PATCH] serial core: fix use-after-free after driver module unload

[sashiko-bot]

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 <johan@kernel.org>

> 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: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>

[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?

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index b06d2649be343..2160e72780b45 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -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);
> +
> +	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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112100541.348390-1-johan@kernel.org?part=1