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

[PATCH v2] 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.

Restore the uart driver module as owner of the tty driver 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().

Note that a few drivers deregister their uart drivers also when a
(platform) device is manually unbound through sysfs (but if you use that
interface, you get to keep the pieces).

Fixes: 2f16669d322e ("TTY: remove re-assignments to tty_driver members")
Cc: stable@vger.kernel.org	# 3.4
Cc: Jiri Slaby <jirislaby@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/

Sashiko noted that this is actually a regression in 3.4.

Johan


Changes in v2:
 - pin the driver module by restoring the tty driver owner field
 - mention that some drivers also deregister their uart drivers on
   manual unbind of a (platform) device through sysfs


 drivers/tty/serial/serial_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2e53d194ecfe..5492bffd3456 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2813,6 +2813,7 @@ int uart_register_driver(struct uart_driver *drv)
 
 	drv->tty_driver = normal;
 
+	normal->owner		= drv->owner;
 	normal->driver_name	= drv->driver_name;
 	normal->name		= drv->dev_name;
 	normal->major		= drv->major;
-- 
2.56.1

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

[sashiko-bot]

> Deregistering a uart driver frees the driver uart state including the
> tty ports themselves.
> 
> Restore the uart driver module as owner of the tty driver 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().
> 
> Note that a few drivers deregister their uart drivers also when a
> (platform) device is manually unbound through sysfs (but if you use that
> interface, you get to keep the pieces).
> 
> Fixes: 2f16669d322e ("TTY: remove re-assignments to tty_driver members")
> Cc: stable@vger.kernel.org	# 3.4
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Sashiko has reviewed this patch and found no issues.

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