tty: port: fix tty_port_tty_vhangup() race

PATCH New Johan Hovold 2 participants 2 messages Sep 10

Subsystems (2)

Stable branch TTY layer and serial drivers

lore

Share & tools

lore

Johan Hovold

author

2026-09-10 12:52 UTC #0 lore raw

PATCH 1/1 tty: port: fix tty_port_tty_vhangup() race

Drivers rely on tty_port_tty_vhangup() to synchronously hang up their ports before tearing down resources as part of deregistration (e.g. on physical disconnect).

If tty_port_tty_vhangup() races with another hangup it may however return before the port has been shut down. This is in turn can lead to use-after-free and NULL-pointer dereferences in racing ioctls (including a synchronous hangup) and in processing of racing asynchronous hangups (carrier loss).

Add the missing serialisation to tty_port_tty_vhangup() and tty_port_hangup() so that the former does not return until the port has been shut down also when there is a racing hangup.

Note that serial core does not use tty_port_hangup(), but it already holds the port mutex when clearing port->tty and shutting down the port in uart_hangup().

Fixes 7ca0ff9ab321 ("tty: Add a full port_close function")

Cc [email protected] # 2.6.32: 2b5eac0f8c6e: tty: introduce and use tty_port_tty_vhangup() helper

Cc [email protected] # 2.6.32

Reported-by [email protected]

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

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

---
 drivers/tty/tty_port.c   | 87 ++++++++++++++++++++++++++--------------
 include/linux/tty_port.h | 22 +---------
 2 files changed, 60 insertions(+), 49 deletions(-)

drivers/tty/tty_port.c +58 ~~-29~~

index ae33987207b6..b22ff9eb7778 100644
@@ -340,19 +340,9 @@ void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty)
 }
 EXPORT_SYMBOL(tty_port_tty_set);
 
-/**
- * tty_port_shutdown - internal helper to shutdown the device
- * @port: tty port to be shut down
- * @tty: the associated tty
- *
- * It is used by tty_port_hangup() and tty_port_close(). Its task is to
- * shutdown the device if it was initialized (note consoles remain
- * functioning). It lowers DTR/RTS (if @tty has HUPCL set) and invokes
- * @port->ops->shutdown().
- */
-static void tty_port_shutdown(struct tty_port *port, struct tty_struct *tty)
+static void tty_port_shutdown_locked(struct tty_port *port, struct tty_struct *tty)
 {
-	guard(mutex)(&port->mutex);
+	lockdep_assert_held(&port->mutex);
 
 	if (port->console)
 		return;

@@ -372,6 +362,23 @@ static void tty_port_shutdown(struct tty_port *port, struct tty_struct *tty)
 		port->ops->shutdown(port);
 }
 
+/**
+ * tty_port_shutdown - internal helper to shutdown the device
+ * @port: tty port to be shut down
+ * @tty: the associated tty
+ *
+ * It is used by tty_port_hangup() and tty_port_close(). Its task is to
+ * shutdown the device if it was initialized (note consoles remain
+ * functioning). It lowers DTR/RTS (if @tty has HUPCL set) and invokes
+ * @port->ops->shutdown().
+ */
+static void tty_port_shutdown(struct tty_port *port, struct tty_struct *tty)
+{
+	guard(mutex)(&port->mutex);
+
+	tty_port_shutdown_locked(port, tty);
+}
+
 /**
  * tty_port_hangup		-	hangup helper
  * @port: tty port

@@ -385,36 +392,58 @@ void tty_port_hangup(struct tty_port *port)
 {
 	struct tty_struct *tty;
 
-	scoped_guard(spinlock_irqsave, &port->lock) {
-		port->count = 0;
-		tty = port->tty;
-		if (tty)
-			set_bit(TTY_IO_ERROR, &tty->flags);
-		port->tty = NULL;
-	}
+	scoped_guard(mutex, &port->mutex) {
+		scoped_guard(spinlock_irqsave, &port->lock) {
+			port->count = 0;
+			tty = port->tty;
+			if (tty)
+				set_bit(TTY_IO_ERROR, &tty->flags);
+			port->tty = NULL;
+		}
 
-	tty_port_set_active(port, false);
-	tty_port_shutdown(port, tty);
+		tty_port_set_active(port, false);
+		tty_port_shutdown_locked(port, tty);
+	}
 	tty_kref_put(tty);
 	wake_up_interruptible(&port->open_wait);
 	wake_up_interruptible(&port->delta_msr_wait);
 }
 EXPORT_SYMBOL(tty_port_hangup);
 
-void __tty_port_tty_hangup(struct tty_port *port, bool check_clocal, bool async)
+/**
+ * tty_port_tty_hangup - helper to hang up a tty asynchronously
+ * @port: tty port
+ * @check_clocal: hang only ttys with %CLOCAL unset?
+ */
+void tty_port_tty_hangup(struct tty_port *port, bool check_clocal)
 {
 	scoped_guard(tty_port_tty, port) {
 		struct tty_struct *tty = scoped_tty();
 
-		if (!check_clocal || !C_CLOCAL(tty)) {
-			if (async)
-				tty_hangup(tty);
-			else
-				tty_vhangup(tty);
-		}
+		if (!check_clocal || !C_CLOCAL(tty))
+			tty_hangup(tty);
+	}
+}
+EXPORT_SYMBOL_GPL(tty_port_tty_hangup);
+
+/**
+ * tty_port_tty_vhangup - helper to hang up a tty synchronously
+ * @port: tty port
+ */
+void tty_port_tty_vhangup(struct tty_port *port)
+{
+	struct tty_struct *tty;
+
+	mutex_lock(&port->mutex);
+	tty = tty_port_tty_get(port);
+	mutex_unlock(&port->mutex);
+
+	if (tty) {
+		tty_vhangup(tty);
+		tty_kref_put(tty);
 	}
 }
-EXPORT_SYMBOL_GPL(__tty_port_tty_hangup);
+EXPORT_SYMBOL_GPL(tty_port_tty_vhangup);
 
 /**
  * tty_port_tty_wakeup - helper to wake up a tty

include/linux/tty_port.h +2 ~~-20~~

index 23cad403bb8f..8d22c59c6f15 100644
@@ -245,7 +245,8 @@ bool tty_port_carrier_raised(struct tty_port *port);
 void tty_port_raise_dtr_rts(struct tty_port *port);
 void tty_port_lower_dtr_rts(struct tty_port *port);
 void tty_port_hangup(struct tty_port *port);
-void __tty_port_tty_hangup(struct tty_port *port, bool check_clocal, bool async);
+void tty_port_tty_hangup(struct tty_port *port, bool check_clocal);
+void tty_port_tty_vhangup(struct tty_port *port);
 void tty_port_tty_wakeup(struct tty_port *port);
 int tty_port_block_til_ready(struct tty_port *port, struct tty_struct *tty,
 		struct file *filp);

@@ -264,25 +265,6 @@ static inline int tty_port_users(struct tty_port *port)
 	return port->count + port->blocked_open;
 }
 
-/**
- * tty_port_tty_hangup - helper to hang up a tty asynchronously
- * @port: tty port
- * @check_clocal: hang only ttys with %CLOCAL unset?
- */
-static inline void tty_port_tty_hangup(struct tty_port *port, bool check_clocal)
-{
-	__tty_port_tty_hangup(port, check_clocal, true);
-}
-
-/**
- * tty_port_tty_vhangup - helper to hang up a tty synchronously
- * @port: tty port
- */
-static inline void tty_port_tty_vhangup(struct tty_port *port)
-{
-	__tty_port_tty_hangup(port, false, false);
-}
-
 #ifdef CONFIG_TTY
 void tty_kref_put(struct tty_struct *tty);
 __DEFINE_CLASS_IS_CONDITIONAL(tty_port_tty, true);

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-10 12:59 UTC #1 lore raw

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 18 lines

Drivers rely on tty_port_tty_vhangup() to synchronously hang up their ports before tearing down resources as part of deregistration (e.g. on physical disconnect).
If tty_port_tty_vhangup() races with another hangup it may however return before the port has been shut down. This is in turn can lead to use-after-free and NULL-pointer dereferences in racing ioctls (including a synchronous hangup) and in processing of racing asynchronous hangups (carrier loss).
Add the missing serialisation to tty_port_tty_vhangup() and tty_port_hangup() so that the former does not return until the port has been shut down also when there is a racing hangup.
Note that serial core does not use tty_port_hangup(), but it already holds the port mutex when clearing port->tty and shutting down the port in uart_hangup().

Quoted 6 lines

Fixes 7ca0ff9ab321 ("tty: Add a full port_close function")
Cc [email protected] # 2.6.32: 2b5eac0f8c6e: tty: introduce and use tty_port_tty_vhangup() helper
Cc [email protected] # 2.6.32
Reported-by [email protected]
Link https://lore.kernel.org/[email protected] open here
Signed-off-by Johan Hovold <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

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