[PATCH] tty: serial: core: fix synchronous external abort in uart_tiocmget() for runtime-suspended ports

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@xxxxxxx>

uart_tiocmget() reads the hardware through ops->get_mctrl(), and the
only check it does first is whether TTY_IO_ERROR is set. That is not
enough to know the port is powered.

On last close, tty_port_close() calls tty_port_shutdown(), which runs
uart_tty_port_shutdown() under port->mutex and powers the port down
with uart_change_pm(UART_PM_STATE_OFF). port->mutex is then dropped,
and only after that does tty_port_close() set TTY_IO_ERROR. A caller
waiting for port->mutex in uart_tiocmget() gets the mutex in between,
sees TTY_IO_ERROR still clear and reads a register of a port that has
already been powered down.

A TIOCMGET ioctl from userspace cannot get there, as it needs an open
file and the file being closed is the last one. But an in-kernel user
that holds the tty through tty_kopen_shared() can, because
tty_kopen_shared() only takes a reference to the tty and does not open
it. ledtrig-tty does this and polls the modem control lines every
~100ms:

  ledtrig_tty_work() -> tty_get_tiocm() -> uart_tiocmget()
    -> ops->get_mctrl() -> register read

On i.MX93 with fsl_lpuart, opening and closing the port in a loop while
the LED trigger is attached to it makes the trigger hit this window
after the LPUART clocks have been gated. The register read in
lpuart32_get_mctrl() then causes a synchronous external abort:

  Internal error: synchronous external abort: 0000000096000010 [#1]  SMP
  Workqueue: events ledtrig_tty_work
  pc : lpuart32_get_mctrl+0x4c/0x64
  Call trace:
   lpuart32_get_mctrl+0x4c/0x64 (P)
   tty_get_tiocm+0x1c/0x30
   ledtrig_tty_work+0x48/0x2dc

Fix this by also checking state->pm_state in uart_tiocmget() and
returning -EIO unless the port is powered on. pm_state is the serial
core record of the port power state. It is only changed under
port->mutex, which uart_tiocmget() already holds, and on close it is
set to UART_PM_STATE_OFF in the same critical section that powers the
port down, before port->mutex is dropped.

Callers that have the port open, such as userspace TIOCMGET and serdev,
are not affected, as pm_state stays UART_PM_STATE_ON while the port is
open. The only exception is system suspend, where uart_suspend_port()
powers the port down and uart_tiocmget() now returns -EIO instead of
touching the hardware.

Fixes: 761ed4a94582 ("tty: serial_core: convert uart_close to use tty_port_close")
Fixes: 6dec659896b4 ("leds: ledtrig-tty: Add additional line state evaluation")
Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
---
 drivers/tty/serial/serial_core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c0deec0e6af4..272feae690ce 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1088,7 +1088,13 @@ static int uart_tiocmget(struct tty_struct *tty)
 	guard(mutex)(&port->mutex);
 
 	uport = uart_port_check(state);
-	if (!uport || tty_io_error(tty))
+	/*
+	 * TTY_IO_ERROR alone does not tell whether the hardware can be
+	 * accessed: on last close it is set only after the port has been
+	 * powered down and port->mutex dropped. Check the power state too.
+	 */
+	if (!uport || tty_io_error(tty) ||
+	    state->pm_state != UART_PM_STATE_ON)
 		return -EIO;
 
 	guard(uart_port_lock_irq)(uport);

---
base-commit: 9a482184d910775cf648863148944945b74e2815
change-id: 20261022-b4-serial_uart_tiocmget-22fea0e6d44f

Best regards,
-- 
Sherry Sun <sherry.sun@xxxxxxx>