[PATCH] serial: imx: cancel RS485 trigger hrtimers in shutdown and remove

[PATCH] serial: imx: cancel RS485 trigger hrtimers in shutdown and remove

[Fan Wu]

The rs485 delay hrtimers trigger_start_tx and trigger_stop_tx are
embedded in the devm allocated struct imx_port, and their callbacks
reach the port through container_of() and touch registers under the
port lock.  Nothing cancels them synchronously: the tx paths only
call hrtimer_try_to_cancel(), which does not wait for a running
callback, and the bounded wait in imx_uart_shutdown() can give up,
force tx_state to OFF, and leave a timer armed.  After
imx_uart_remove() returns, devm frees the port and a late callback
dereferences freed memory.

Cancel both timers at the end of imx_uart_shutdown(), after the port
lock is dropped and before the clocks are disabled, and again in
imx_uart_remove() before the devm free: serial core does not call the
driver shutdown on every path that reaches remove().

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

Fixes: bd78ecd6056d ("serial: imx: use hrtimers for rs485 delays")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/tty/serial/imx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 251a50c8aa38..86c99f73c50a 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1707,6 +1707,10 @@ static void imx_uart_shutdown(struct uart_port *port)
 
 	uart_port_unlock_irqrestore(&sport->port, flags);
 
+	/* The rs485 trigger callbacks take the port lock and touch registers. */
+	hrtimer_cancel(&sport->trigger_start_tx);
+	hrtimer_cancel(&sport->trigger_stop_tx);
+
 	clk_disable_unprepare(sport->clk_per);
 	clk_disable_unprepare(sport->clk_ipg);
 }
@@ -2649,6 +2653,10 @@ static void imx_uart_remove(struct platform_device *pdev)
 	struct imx_port *sport = platform_get_drvdata(pdev);
 
 	uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
+
+	/* Serial core can reach remove() without calling the driver shutdown. */
+	hrtimer_cancel(&sport->trigger_start_tx);
+	hrtimer_cancel(&sport->trigger_stop_tx);
 }
 
 static void imx_uart_restore_context(struct imx_port *sport)

Re: [PATCH] serial: imx: cancel RS485 trigger hrtimers in shutdown and remove

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing cancellation of `sport->timer` and modem status GPIO IRQs in `imx_uart_remove()` leads to a use-after-free and system crash (bus fault) for console ports.
--

--- Patch [1]: [PATCH] serial: imx: cancel RS485 trigger hrtimers in shutdown and remove ---
commit 00c3fef931cf8afdbbbb1c6e8dc49797a632d082
Author: Fan Wu <fanwu01@zju.edu.cn>

serial: imx: cancel RS485 trigger hrtimers in shutdown and remove

This commit cancels the RS485 trigger hrtimers synchronously in
imx_uart_shutdown() and imx_uart_remove() to prevent use-after-free
after the devm-allocated port is freed.

> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 251a50c8aa387..86c99f73c50a3 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c

[ ... ]

> @@ -2649,6 +2653,10 @@ static void imx_uart_remove(struct platform_device *pdev)
>  	struct imx_port *sport = platform_get_drvdata(pdev);
>  
>  	uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
> +
> +	/* Serial core can reach remove() without calling the driver shutdown. */
> +	hrtimer_cancel(&sport->trigger_start_tx);
> +	hrtimer_cancel(&sport->trigger_stop_tx);

[Severity: High]
This is a pre-existing issue, but since we are cancelling timers in
imx_uart_remove(), does this function also need to cancel sport->timer and
disable the modem status GPIO IRQs?

If the system boots with hardware flow control enabled on the console,
imx_uart_console_setup() sets termios, which calls imx_uart_enable_ms() and
arms sport->timer.

If the driver is unbound via sysfs or the module is unloaded,
imx_uart_remove() unregisters the console, disabling the UART clocks.
However, it does not cancel sport->timer or disable the MS GPIO interrupts.

When devres frees the imx_port structure, the timer can still fire:

drivers/tty/serial/imx.c:imx_uart_timeout() {
        struct imx_port *sport = timer_container_of(sport, t, timer);
        unsigned long flags;

        if (sport->port.state) {
                uart_port_lock_irqsave(&sport->port, &flags);
                imx_uart_mctrl_check(sport);
                uart_port_unlock_irqrestore(&sport->port, flags);

                mod_timer(&sport->timer, jiffies + MCTRL_TIMEOUT);
        }
}

Could this lead to accessing the freed sport struct and reading disabled
UART registers, resulting in a use-after-free and system crash?

>  }
>  
>  static void imx_uart_restore_context(struct imx_port *sport)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819021916.442827-1-fanwu01@zju.edu.cn?part=1

Re: [PATCH] serial: imx: cancel RS485 trigger hrtimers in shutdown and remove

[Jiri Slaby]

On 19. 08. 26, 4:19, Fan Wu wrote:
> The rs485 delay hrtimers trigger_start_tx and trigger_stop_tx are
> embedded in the devm allocated struct imx_port, and their callbacks
> reach the port through container_of() and touch registers under the
> port lock.  Nothing cancels them synchronously: the tx paths only
> call hrtimer_try_to_cancel(), which does not wait for a running
> callback, and the bounded wait in imx_uart_shutdown() can give up,
> force tx_state to OFF, and leave a timer armed.  After
> imx_uart_remove() returns, devm frees the port and a late callback
> dereferences freed memory.
> 
> Cancel both timers at the end of imx_uart_shutdown(), after the port
> lock is dropped and before the clocks are disabled, and again in
> imx_uart_remove() before the devm free:


> serial core does not call the
> driver shutdown on every path that reaches remove().

Could you be more specific on what path it does not?

thanks,
-- 
js
suse labs

Re: [PATCH] serial: imx: cancel RS485 trigger hrtimers in shutdown and remove

[Fan Wu]

> On Aug 19, 2026, at 12:26, Jiri Slaby <jirislaby@kernel.org> wrote:
> 
>> serial core does not call the
>> driver shutdown on every path that reaches remove().
> 
> Could you be more specific on what path it does not?
> 
> thanks,
> -- 
> js
> suse labs

Hi Jiri

Yes. The relevant path is a console port whose last TTY user closes it
before the device is unbound.

tty_port_shutdown() skips port->ops->shutdown() for console ports. The
last close then clears the active state and dissociates the TTY, so the
later tty_port_tty_vhangup() in the remove path has no TTY to hang up
and cannot invoke the driver shutdown.  That is the path the cancel in
imx_uart_remove() covers.

trigger_stop_tx can already be pending at that point: after TXDC,
imx_uart_stop_tx() starts it for delay_rts_after_send, whereas the close
path only waits for TXDC.

I can add this to the changelog if you would prefer the rationale to be
spelled out there.

Thanks,
Fan