[PATCH] serial: fix ioctl hangup race

[PATCH] serial: fix ioctl hangup race

[Johan Hovold]

The tty ioctls can race with hangup and end up calling into a tty
driver for a device that is already gone or powered down.

Add the missing checks to make sure the port has not been hung up before
accessing the hardware to avoid issues like kernel panic due to
unclocked accesses.

Note that TIOCGSERIAL, TIOCGICOUNT and TIOCMIWAIT do not access hardware
and are therefore not affected by the race.

Fixes: 04f378b198da ("tty: BKL pushdown")
Cc: stable@vger.kernel.org	# 2.6.26
Signed-off-by: Johan Hovold <johan@kernel.org>
---

I've just sent a fix for USB serial here:

	https://lore.kernel.org/r/20260903163146.1498497-1-johan@kernel.org

and will take closer look at the other TTY drivers tomorrow.

Johan


 drivers/tty/serial/serial_core.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 95774b0f1484..aac12be4ceac 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -896,7 +896,7 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
 	upf_t old_flags, new_flags;
 	int retval;
 
-	if (!uport)
+	if (!uport || tty_io_error(tty))
 		return -EIO;
 
 	new_port = new_info->port;
@@ -1119,7 +1119,7 @@ static int uart_break_ctl(struct tty_struct *tty, int break_state)
 	guard(mutex)(&port->mutex);
 
 	uport = uart_port_check(state);
-	if (!uport)
+	if (!uport || tty_io_error(tty))
 		return -EIO;
 
 	if (uport->type != PORT_UNKNOWN && uport->ops->break_ctl)
@@ -1144,7 +1144,7 @@ static int uart_do_autoconfig(struct tty_struct *tty, struct uart_state *state)
 	 */
 	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &port->mutex) {
 		uport = uart_port_check(state);
-		if (!uport)
+		if (!uport || tty_io_error(tty))
 			return -EIO;
 
 		if (tty_port_users(port) != 1)
@@ -1646,7 +1646,7 @@ static void uart_set_termios(struct tty_struct *tty,
 	guard(mutex)(&state->port.mutex);
 
 	uport = uart_port_check(state);
-	if (!uport)
+	if (!uport || tty_io_error(tty))
 		return;
 
 	/*
@@ -1798,7 +1798,11 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
 	 * 'timeout' / 'expire' give us the maximum amount of time
 	 * we wait.
 	 */
-	while (!port->ops->tx_empty(port)) {
+	for (;;) {
+		scoped_guard(mutex, &state->port.mutex) {
+			if (tty_io_error(tty) || port->ops->tx_empty(port))
+				break;
+		}
 		msleep_interruptible(jiffies_to_msecs(char_time));
 		if (signal_pending(current))
 			break;
-- 
2.55.0

Re: [PATCH] serial: fix ioctl hangup race

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Using a `break` statement inside `scoped_guard()` incorrectly exits the macro's internal loop instead of the intended outer `for (;;)` loop, leading to an infinite wait or severe latency.

Pre-existing issues:
- [High] The TIOCMIWAIT ioctl is still vulnerable to the hangup race because `uart_wait_modem_status()` accesses hardware registers via `uart_enable_ms()`, despite the commit message claiming otherwise.
--

commit cc61eb3f9ab972ab6ba3e89cd9d7a45856a37c6c
Author: Johan Hovold <johan@kernel.org>

serial: fix ioctl hangup race

This patch attempts to prevent tty ioctls from racing with hangup by
checking if the port has been hung up before accessing the hardware. It
adds missing checks for tty_io_error() across several ioctl paths.

Link: https://lore.kernel.org/r/20260903163146.1498497-1-johan@kernel.org

> serial: fix ioctl hangup race
>
> The tty ioctls can race with hangup and end up calling into a tty
> driver for a device that is already gone or powered down.
>
> Add the missing checks to make sure the port has not been hung up before
> accessing the hardware to avoid issues like kernel panic due to
> unclocked accesses.
>
> Note that TIOCGSERIAL, TIOCGICOUNT and TIOCMIWAIT do not access hardware
> and are therefore not affected by the race.

[Severity: High]
This is a pre-existing issue, but does TIOCMIWAIT actually avoid hardware
access?

If a userspace process calls the TIOCMIWAIT ioctl, it routes through
uart_ioctl() to uart_wait_modem_status(). In uart_wait_modem_status(), we
directly call uart_enable_ms(), which invokes the enable_ms() hardware
callback to write to registers.

If a concurrent hangup powers down the port before uart_enable_ms()
executes, could this lead to an unclocked hardware access on a
powered-down port?

[ ... ]

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 95774b0f1484b..aac12be4ceac6 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c

[ ... ]

> @@ -1798,7 +1798,11 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
>  	 * 'timeout' / 'expire' give us the maximum amount of time
>  	 * we wait.
>  	 */
> -	while (!port->ops->tx_empty(port)) {
> +	for (;;) {
> +		scoped_guard(mutex, &state->port.mutex) {
> +			if (tty_io_error(tty) || port->ops->tx_empty(port))
> +				break;

[Severity: High]
Does this break statement exit the intended outer loop?

Since scoped_guard() is implemented as a hidden for loop in
include/linux/cleanup.h, the break statement will only exit the
scoped_guard macro's internal loop. The outer for (;;) loop in
uart_wait_until_sent() will continue executing and unconditionally sleep
via msleep_interruptible().

If timeout is 0, won't this result in an infinite wait loop unless a
signal is pending?

> +		}
>  		msleep_interruptible(jiffies_to_msecs(char_time));
>  		if (signal_pending(current))
>  			break;

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

Re: [PATCH] serial: fix ioctl hangup race

[Johan Hovold]

On Thu, Sep 03, 2026 at 04:45:31PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Using a `break` statement inside `scoped_guard()` incorrectly
> exits the macro's internal loop instead of the intended outer `for
> (;;)` loop, leading to an infinite wait or severe latency.

Bah. Another reason to dislike guards.

> Pre-existing issues:
> - [High] The TIOCMIWAIT ioctl is still vulnerable to the hangup race
> because `uart_wait_modem_status()` accesses hardware registers via
> `uart_enable_ms()`, despite the commit message claiming otherwise.

I missed that call. Will revisit.

Johan