Re: [PATCH v2 2/2] serial: tegra: fix TX DMA descriptor use-after-free

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] serial: tegra: UAF and double xmit advance in tx_dma_complete
- [High] serial: tegra: UAF in tegra_uart_stop_tx() due to stale tx_in_progress after flush
--

commit 7f4e5d2d923132e41e93a227ae34dba2a8eda9f6
Author: Austin Schlegel <schlegelas@outlook.com>

serial: tegra: fix TX DMA descriptor use-after-free

This patch moves async_tx_ack() before dmaengine_terminate_all() in
tegra_uart_stop_tx() to prevent a use-after-free when the DMA descriptor is
immediately freed by the termination.

> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index a23a92cebe0df..306a38b2fb1e7 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -639,9 +639,9 @@ static void tegra_uart_stop_tx(struct uart_port *u)
>  
>  	dmaengine_pause(tup->tx_dma_chan);
>  	dmaengine_tx_status(tup->tx_dma_chan, tup->tx_cookie, &state);
> +	async_tx_ack(tup->tx_dma_desc);

[Severity: High]
Does this create a use-after-free if tegra_uart_flush_buffer() runs first?

If tegra_uart_flush_buffer() is called via the TCFLSH ioctl, it terminates
the DMA channel and frees the descriptor:

drivers/tty/serial/serial-tegra.c:tegra_uart_flush_buffer() {
	...
	if (tup->tx_dma_chan)
		dmaengine_terminate_all(tup->tx_dma_chan);
}

But it fails to clear tup->tx_in_progress. If tegra_uart_stop_tx() is then
called, it bypasses the tx_in_progress check and calls async_tx_ack() on
the freed tx_dma_desc.

>  	dmaengine_terminate_all(tup->tx_dma_chan);

[Severity: High]
Can this termination race with tegra_uart_tx_dma_complete() and cause a UAF
and double accounting?

tegra_uart_tx_dma_complete() runs asynchronously and accesses tup->tx_dma_desc
outside of the UART port lock:

drivers/tty/serial/serial-tegra.c:tegra_uart_tx_dma_complete() {
	...
	async_tx_ack(tup->tx_dma_desc);
	uart_port_lock_irqsave(&tup->uport, &flags);
	uart_xmit_advance(&tup->uport, count);
	...
}

tegra_uart_stop_tx() runs under the port lock and terminates the DMA channel,
which immediately frees the descriptor. If tegra_uart_stop_tx() runs
concurrently after tegra_uart_tx_dma_complete() starts but before it acquires
the lock, the completion callback will trigger a use-after-free on the
descriptor.

Furthermore, both functions calculate count and advance the FIFO. If they
race, tegra_uart_tx_dma_complete() will blindly call uart_xmit_advance() a
second time inside the lock, skipping untransmitted characters and corrupting
the ring buffer state.

>  	count = tup->tx_bytes_requested - state.residue;
> -	async_tx_ack(tup->tx_dma_desc);
>  	uart_xmit_advance(&tup->uport, count);
>  	tup->tx_in_progress = 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-serial-tegra-dma-uaf-v2-0-b142b31b67e3@outlook.com?part=2