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

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

[Austin via B4 Relay]

Both patches in this series fix the same bug pattern in
drivers/tty/serial/serial-tegra.c: dmaengine_terminate_all() is called
before async_tx_ack() on the DMA descriptor it just terminated. With
the GPC DMA driver (tegra186-gpc-dma.c, used on Tegra194/Tegra234),
terminating a channel frees the active descriptor synchronously via
vchan_dma_desc_free_list(), so the subsequent ack touches freed
memory. dmaengine clients are not allowed to touch a descriptor once
it has been terminated.

Patch 1/2 (RX) was found via a KFENCE use-after-free report on a
Jetson AGX Orin (Tegra234) under UART loopback traffic, and the fix
has since been soak tested for 24+ hours with
kfence.sample_interval=1 with zero further reports.

Patch 2/2 (TX) is the same ordering bug in the analogous TX path
(tegra_uart_stop_tx()), found by code inspection once the RX bug was
understood. It has not been reproduced on hardware - triggering it
requires stopping an in-flight TX DMA transfer at the right moment,
which did not occur during RX testing - but it is the identical
dmaengine_terminate_all()-then-ack pattern in the same driver against
the same DMA engine, so I'm sending it alongside the RX fix rather
than leaving it unfixed.

Both patches keep the pre-existing descriptor-leak fix from commit
b31245b94207 ("serial: tegra: ack the rx dma desc after transfer
terminated") intact: the ack still happens for every terminated
transfer, just before the descriptor is freed instead of after.

Testing was done on Tegra234 (Jetson AGX Orin) only.

Signed-off-by: Austin <austin.schlegel@arthrex.com>
---
Austin (2):
      serial: tegra: fix RX DMA descriptor use-after-free
      serial: tegra: fix TX DMA descriptor use-after-free

 drivers/tty/serial/serial-tegra.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
---
base-commit: 5dad87615c9861cfa366ca984b52f581e861df20
change-id: 20261001-tty-linus-ae90fc64265d

Best regards,
-- 
Austin <austin.schlegel@arthrex.com>

[PATCH 1/2] serial: tegra: fix RX DMA descriptor use-after-free

[Austin via B4 Relay]

From: Austin <austin.schlegel@arthrex.com>

tegra_uart_terminate_rx_dma() calls dmaengine_terminate_all() and then
tegra_uart_rx_buffer_push(), which calls async_tx_ack(tup->rx_dma_desc)
on the just-terminated descriptor. With the GPC DMA driver,
dmaengine_terminate_all() frees the active descriptor immediately, so
the subsequent async_tx_ack() touches freed memory. dmaengine clients
are not allowed to touch a descriptor once it has been terminated.

The same freed-descriptor access happens via tegra_uart_rx_dma_complete(),
which also reaches tegra_uart_rx_buffer_push() after RX has stopped.

Move the ack into the two call sites that still own a live descriptor:
tegra_uart_rx_dma_complete() (before the completion callback hands the
descriptor back) and tegra_uart_terminate_rx_dma() (before terminating),
and drop it from tegra_uart_rx_buffer_push() itself, since that function
no longer has a descriptor it's safe to ack.

The ack was originally added in commit b31245b94207 ("serial: tegra:
ack the rx dma desc after transfer terminated") to avoid a descriptor
leak with the Tegra20 APB DMA driver. Keep that fix intact: the ack
still happens for every terminated RX transfer, just before the
descriptor is freed instead of after.

Found by inspection while chasing the KFENCE report below on a Jetson
AGX Orin (Tegra234) with UART loopback:

BUG: KFENCE: use-after-free read in tegra_uart_rx_buffer_push+0x38/0x168
 tegra_uart_rx_buffer_push+0x38/0x168
 tegra_uart_terminate_rx_dma+0x88/0xf8
 tegra_uart_isr+0x380/0x470

allocated by task 0 on cpu 0 (~9 ms earlier):
 tegra_dma_prep_slave_sg+0x134/0x3c0
 tegra_uart_start_rx_dma.isra.0+0xc4/0x138
 tegra_uart_isr+0x340/0x470

freed by task 0 on cpu 0:
 tegra_dma_desc_free+0x1c/0x30
 vchan_dma_desc_free_list+0x10c/0x160
 tegra_dma_terminate_all+0x21c/0x290
 tegra_uart_terminate_rx_dma+0x7c/0xf8

Soak tested on Tegra234 (Jetson AGX Orin) for 13+ hours with
kfence.sample_interval=1 and continuous UART loopback traffic: zero
KFENCE reports, tx/rx byte counts in /proc/tty/driver/tegra_hsuart
remained equal throughout with no framing or break errors.

Fixes: b31245b94207 ("serial: tegra: ack the rx dma desc after transfer terminated")
Signed-off-by: Austin <austin.schlegel@arthrex.com>
---
 drivers/tty/serial/serial-tegra.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 8004fc00fb9c..c9ec633e7164 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -716,7 +716,6 @@ static void tegra_uart_rx_buffer_push(struct tegra_uart_port *tup,
 	struct tty_port *port = &tup->uport.state->port;
 	unsigned int count;
 
-	async_tx_ack(tup->rx_dma_desc);
 	count = tup->rx_bytes_requested - residue;
 
 	/* If we are here, DMA is stopped */
@@ -747,6 +746,7 @@ static void tegra_uart_rx_dma_complete(void *args)
 		set_rts(tup, false);
 
 	tup->rx_dma_active = false;
+	async_tx_ack(tup->rx_dma_desc);
 	tegra_uart_rx_buffer_push(tup, 0);
 	tegra_uart_start_rx_dma(tup);
 
@@ -769,6 +769,7 @@ static void tegra_uart_terminate_rx_dma(struct tegra_uart_port *tup)
 
 	dmaengine_pause(tup->rx_dma_chan);
 	dmaengine_tx_status(tup->rx_dma_chan, tup->rx_cookie, &state);
+	async_tx_ack(tup->rx_dma_desc);
 	dmaengine_terminate_all(tup->rx_dma_chan);
 
 	tegra_uart_rx_buffer_push(tup, state.residue);

-- 
2.53.0

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

[Austin via B4 Relay]

From: Austin <austin.schlegel@arthrex.com>

tegra_uart_stop_tx() calls dmaengine_terminate_all() and then
async_tx_ack(tup->tx_dma_desc) on the descriptor that was just
terminated. With the GPC DMA driver, dmaengine_terminate_all() frees
the active descriptor immediately, so the ack call that follows
touches freed memory, the same use-after-free pattern fixed for the
RX path in tegra_uart_terminate_rx_dma() ("serial: tegra: fix RX DMA
descriptor use-after-free"). dmaengine clients must not touch a
descriptor once dmaengine_terminate_all() has returned.

Move the ack before dmaengine_terminate_all(), while the descriptor
is still owned by the driver. tegra_uart_tx_dma_complete() already
acks inside the completion callback, where the descriptor is valid,
and is unaffected.

Found by code inspection while fixing the analogous RX bug; not
reproduced on hardware, since triggering it requires stopping an
in-flight TX DMA transfer (e.g. via a modem control line or flush)
at the right moment. The RX and TX paths share the same
dmaengine_terminate_all()-then-ack structure and the same root cause.

Signed-off-by: Austin <austin.schlegel@arthrex.com>
---
 drivers/tty/serial/serial-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index c9ec633e7164..0d1a2ebe2a59 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -625,9 +625,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);
 	dmaengine_terminate_all(tup->tx_dma_chan);
 	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;
 }

-- 
2.53.0

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

[Austin Schlegel]

> tegra_uart_terminate_rx_dma() calls dmaengine_terminate_all() and then
> tegra_uart_rx_buffer_push(), which calls async_tx_ack(tup->rx_dma_desc)
> ...
> The same freed-descriptor access happens via tegra_uart_rx_dma_complete(),
> which also reaches tegra_uart_rx_buffer_push() after RX has stopped.

This second paragraph is wrong - the descriptor is still valid inside
tegra_uart_rx_dma_complete(); the ack only moves there because it's
removed from tegra_uart_rx_buffer_push(). I'll fix this in v2.

> Soak tested on Tegra234 (Jetson AGX Orin) for 13+ hours with
> kfence.sample_interval=1 and continuous UART loopback traffic...

This should state 24+ hours, I'll update the number in v2.
This e-mail and any files transmitted with it are the property of Arthrex, Inc. and/or its affiliates, are confidential, and are intended solely for the use of the individual or entity to whom this e-mail is addressed. If you are not one of the named recipient(s) or otherwise have reason to believe that you have received this message in error, please notify the sender at 239-598-4302 and delete this message immediately from your computer. Any other use, retention, dissemination forwarding, printing or copying of this e-mail is strictly prohibited. Please note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. Finally, while Arthrex uses virus protection, the recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email.

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

[Austin Schlegel]

> Found by code inspection while fixing the analogous RX bug; not
> reproduced on hardware, since triggering it requires stopping an
> in-flight TX DMA transfer (e.g. via a modem control line or flush)
> at the right moment...

tegra_uart_stop_tx() is reached when TX is stopped by flow control
(e.g. CTS deasserting), not by a flush - flush_buffer() does not go
through this path. I'll remove that example in v2.
This e-mail and any files transmitted with it are the property of Arthrex, Inc. and/or its affiliates, are confidential, and are intended solely for the use of the individual or entity to whom this e-mail is addressed. If you are not one of the named recipient(s) or otherwise have reason to believe that you have received this message in error, please notify the sender at 239-598-4302 and delete this message immediately from your computer. Any other use, retention, dissemination forwarding, printing or copying of this e-mail is strictly prohibited. Please note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. Finally, while Arthrex uses virus protection, the recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email.