[PATCH] tty: serial: amba-pl011: cancel RS485 trigger hrtimers before removal

Fan Wu posted 1 patch 3 days, 20 hours ago
drivers/tty/serial/amba-pl011.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
[PATCH] tty: serial: amba-pl011: cancel RS485 trigger hrtimers before removal
Posted by Fan Wu 3 days, 20 hours ago
pl011_shutdown() calls pl011_rs485_tx_stop() when RS485 is enabled and
the transmit state machine is still active.  That function may re-arm
the trigger_stop_tx hrtimer via hrtimer_start() when the TX FIFO is not
yet empty, or when delay_rts_after_send is configured, and the trigger
timers can equally be left armed from a prior pl011_rs485_tx_start().

On removal, uart_remove_one_port() can finish without draining the RS485
hrtimers, and devm frees the uart_amba_port once pl011_remove() returns.
The existing teardown syncs do not help here: pl011_disable_interrupts()
only masks the UART IMSC register, pl011_dma_shutdown() cancels a
different (DMA rx) timer_list, and free_irq() plus serial_core's
synchronize_irq() only drain the UART interrupt handler, not the hrtimer
softirq callbacks.  Either trigger callback can therefore recover uap
from its embedded hrtimer and access uap->port after the free.

Cancel both RS485 trigger hrtimers in pl011_shutdown(), after the
re-arming pl011_rs485_tx_stop() call and before free_irq(), and again
in pl011_remove() after uart_remove_one_port().  The remove-side
calls cover paths such as suspend followed by unbind, where serial
core need not invoke the driver's shutdown callback.  The
callbacks pl011_trigger_start_tx and pl011_trigger_stop_tx return
HRTIMER_NORESTART, so hrtimer_cancel() fully drains any pending and
concurrently running callback without a self-restart loop.  As
pl011_trigger_start_tx() may re-enable the TX interrupt via
pl011_start_tx(), call pl011_disable_interrupts() again after the
cancels to re-mask IMSC before free_irq().  The cancel site does not
hold uart_port_lock across the cancel
(pl011_disable_interrupts() already released it) and the callbacks take
that lock internally, so there is no self-deadlock.  This mirrors the
RS485 teardown pattern in 8250_port.c (serial8250_em485_destroy),
which cancels both hrtimers before the embedding object is freed.

This issue was found by an in-house static analysis tool.

Fixes: 2c1fd53af21b ("serial: amba-pl011: Fix RTS handling in RS485 mode")
Cc: stable@vger.kernel.org   # v6.14+
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/tty/serial/amba-pl011.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8ed91e1da22b..021b542b3d99 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2083,6 +2083,16 @@ static void pl011_shutdown(struct uart_port *port)
 	if ((port->rs485.flags & SER_RS485_ENABLED && uap->rs485_tx_state != OFF))
 		pl011_rs485_tx_stop(uap);
 
+	/*
+	 * pl011_rs485_tx_stop() above may have (re-)armed the RS485 trigger
+	 * hrtimers.  A callback may also re-enable the TX interrupt, so
+	 * cancel them and mask interrupts again before freeing the IRQ.
+	 * pl011_remove() cancels them again before freeing the owner.
+	 */
+	hrtimer_cancel(&uap->trigger_start_tx);
+	hrtimer_cancel(&uap->trigger_stop_tx);
+	pl011_disable_interrupts(uap);
+
 	free_irq(uap->port.irq, uap);
 
 	pl011_disable_uart(uap);
@@ -3063,6 +3073,8 @@ static void pl011_remove(struct amba_device *dev)
 	struct uart_amba_port *uap = amba_get_drvdata(dev);
 
 	uart_remove_one_port(&amba_reg, &uap->port);
+	hrtimer_cancel(&uap->trigger_start_tx);
+	hrtimer_cancel(&uap->trigger_stop_tx);
 	pl011_unregister_port(uap);
 }
 
-- 
2.34.1