Re: [PATCH 08/24] serial: 8250: cancel overrun backoff work on port shutdown and unregister

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3701 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> The overrun_backoff delayed work can be scheduled from IRQ handlers in
> 8250_fsl.c and 8250_omap.c via schedule_delayed_work(). However, this
> work is never canceled in the shutdown or unregister paths, so it can
> fire after the port has been torn down, causing stale hardware access or
> use-after-free.
> 
> The existing synchronize_irq() calls in the shutdown path only
> synchronize with the IRQ handler itself, not with workqueue callbacks
> that the handler has already enqueued. Add cancel_delayed_work_sync()
> after the IRQ is quiesced in serial8250_do_shutdown() (the generic
> shutdown path used by FSL and others), omap_8250_shutdown() (the
> OMAP-specific shutdown path), and serial8250_unregister_port() (the
> module unload / hot-unplug path).
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Joshua Rogers <linux@joshua.hu>
> Cc: Fushuai Wang <wangfushuai@baidu.com>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: Moteen Shah <m-shah@ti.com>
> Cc: Kendall Willis <k-willis@ti.com>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_core.c | 3 +++
>  drivers/tty/serial/8250/8250_omap.c | 3 +++
>  drivers/tty/serial/8250/8250_port.c | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 5730ef97f836..af94ed4e633e 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -950,6 +950,9 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
>  
>  	serial8250_em485_free(uart);
>  
> +	if (uart->overrun_backoff_time_ms)
> +		cancel_delayed_work_sync(&uart->overrun_backoff);
> +
>  	uart_remove_one_port(&serial8250_reg, &uart->port);
>  	if (serial8250_isa_devs) {
>  		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 29a203ed93fd..07ebed6ac6c9 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -786,6 +786,9 @@ static void omap_8250_shutdown(struct uart_port *port)
>  
>  	disable_irq_nosync(port->irq);
>  
> +	if (up->overrun_backoff_time_ms)
> +		cancel_delayed_work_sync(&up->overrun_backoff);
> +
>  	serial8250_release_dma(up);
>  	up->dma = NULL;
>  
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 4afebd96e062..710d2a28b918 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2497,6 +2497,9 @@ void serial8250_do_shutdown(struct uart_port *port)
>  
>  	synchronize_irq(port->irq);
>  
> +	if (up->overrun_backoff_time_ms)
> +		cancel_delayed_work_sync(&up->overrun_backoff);

Wouldn't it be better for fsl to add own shutdown function to do this 
there and not within the generic code?

This entire overrun_backoff machinery seems something that should be part 
of the respective drivers but that's beyond the scope of this fix.

> +
>  	if (up->dma)
>  		serial8250_release_dma(up);
>  
> 
> 

-- 
 i.