Re: [PATCH v8 7/9] serial: max310x: schedule tx_work directly from the IRQ handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tty: max310x: Use-After-Free of workqueues in probe error path and remove
--

commit 6bfa220718efcbb6292bc06e4e18a373318c9c1a
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

serial: max310x: schedule tx_work directly from the IRQ handler

This commit schedules tx_work directly from the IRQ handler instead of going
through start_tx(). This keeps the interrupt path as a pure FIFO refill in
preparation for a following patch.

> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 0c40283f44ba1..c1fe4ad887839 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
> @@ -858,8 +858,12 @@ static irqreturn_t max310x_port_irq(struct max310x_port *s, int portno)
>  		}
>  		if (rxlen)
>  			max310x_handle_rx(port, rxlen);
> +		/*
> +		 * TXEMPTY latches on the FIFO becoming empty, so a stale
> +		 * interrupt cannot pump data during an RTS setup delay.
> +		 */
>  		if (ists & MAX310X_IRQ_TXEMPTY_BIT)
> -			max310x_start_tx(port);
> +			schedule_work(&s->p[portno].tx_work);
>  	} while (1);
>  
>  	return res;

[Severity: High]
Does this code allow delayed works to outlive the driver's memory?

In max310x_remove(), works are cancelled before uart_remove_one_port():

drivers/tty/serial/max310x.c:max310x_remove() {
	for (i = 0; i < s->devtype->nr; i++) {
		cancel_work_sync(&s->p[i].tx_work);
		cancel_work_sync(&s->p[i].md_work);
		cancel_work_sync(&s->p[i].rs_work);

		if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
			uart_remove_one_port(&max310x_uart, &s->p[i].port);
		...
	}
}

If the port is open, removing the port invokes a vhangup, dropping DTR/RTS via
uart_shutdown() -> uart_port_dtr_rts(0) -> max310x_set_mctrl().

This synchronously re-schedules md_work. Since it was already cancelled
earlier, it is left pending while max310x_remove() completes and devres
frees the max310x_port structure, resulting in a use-after-free.

Similarly, the probe error path completely fails to cancel works before devres
frees the structure:

drivers/tty/serial/max310x.c:max310x_probe() {
...
out_uart:
	for (i = 0; i < devtype->nr; i++) {
		if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
			uart_remove_one_port(&max310x_uart, &s->p[i].port);
	}
...
}

This means md_work and rs_work scheduled by uart_add_one_port() will execute
after devres cleanup on probe failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-max310x-rs485-sw-delay-v8-0-6ba881f81e1f@vaisala.com?part=7