[BUG] serial: amba-pl011, imx, fsl_lpuart: ->flush_buffer() unmaps the TX DMA buffer before the transfer has stopped

[BUG] serial: amba-pl011, imx, fsl_lpuart: ->flush_buffer() unmaps the TX DMA buffer before the transfer has stopped

[Samarth Narang]

Hi,

I found three serial drivers that unmap their TX DMA buffers in
->flush_buffer() without first ensuring that the outstanding DMA
transfer has stopped. Line numbers refer to mainline at 83f32c773255.

uart_flush_buffer() invokes ->flush_buffer() under the port lock.
The path is reachable through TCFLSH(TCOFLUSH), hangup, and close, so a
userspace process with access to the port can trigger it while TX DMA
is active.

Because the port lock is held, the flush hook cannot use
dmaengine_terminate_sync(). However, dmaengine_terminate_async() only
requests termination; it does not guarantee that DMA has stopped or
that its completion callback has finished. The DMAengine API requires
synchronization before memory still accessible to the descriptor is
released (include/linux/dmaengine.h and
Documentation/driver-api/dmaengine/client.rst).

I found the following cases:

1. amba-pl011

pl011_dma_flush_buffer() (lines 858-877) calls
dmaengine_terminate_async() and immediately unmaps dmatx.buf.

The unmap can therefore occur before the DMA transfer and its callback
have finished.

2. imx

imx_uart_flush_buffer() (lines 1716-1739) calls the deprecated
dmaengine_terminate_all() and, when dma_is_txing is set, immediately
unmaps tx_sgl at line 1729.

The imx-sdma termination path disables the channel but defers descriptor
cleanup to a worker that sleeps for 1-2 ms because the SDMA core may
still be running. Consequently, termination does not establish that
the controller has stopped accessing the mapping when the unmap occurs.

There is also a separate callback race. imx_uart_dma_tx_callback()
unconditionally unmaps tx_sgl at line 595. If the callback is already
running when the flush begins, for example while waiting to acquire
the port lock, it can subsequently unmap the same scatterlist again
and may attempt to start another transfer.

3. fsl_lpuart

lpuart_flush_buffer() (lines 601-614) unmaps tx_sgl at line 609
before calling dmaengine_terminate_async() at line 613.

In this case, the mapping is released before termination has even
been requested, while eDMA may still be reading the buffer.

Potential consequences

These paths can release a DMA mapping while the previous transfer
may still retain read access to its backing storage.

Without an IOMMU or SWIOTLB, a late read can race with subsequent
modification or reuse of the buffer. With an IOMMU, an unmapped IOVA
may subsequently be reassigned. With SWIOTLB bouncing, a released
bounce slot may be reused by another mapping. If the old transfer
continues reading after such reuse, data belonging to a different
operation could be transmitted over the serial interface.

LS1028A is one configuration worth examining for the SWIOTLB case:
fsl-edma uses a 32-bit DMA mask without FSL_EDMA_DRV_TCD64, while
supported systems may have RAM above 4 GiB. Whether the affected TX
buffers actually use bouncing, and whether a late read occurs after
slot reuse, still needs to be established.

A related issue in qcom-geni was fixed by e3c04834ae1a
("serial: qcom-geni: fix TX DMA buffer flush"), associated with
CVE-2026-74655.

Possible fix

Since ->flush_buffer() runs under the port lock and cannot sleep,
one approach would be to:

- request asynchronous termination and mark the current TX operation
  as flushing, without immediately unmapping its buffer;
- defer synchronization and mapping teardown to a sleepable context;
- ensure that a completion callback for the flushed operation cannot
  unmap the buffer again, advance stale TX state, or restart DMA; and
- ensure that shutdown, and suspend where applicable, drain the deferred
  cleanup before releasing the associated resources.

The callback and worker would need to coordinate ownership of the
mapping, and any work draining would need to occur outside locks
required by the callback.

I have compile-tested candidate patches for amba-pl011 and
fsl_lpuart using this general approach. I can send them for review,
or adapt them if the maintainers prefer a different synchronization
strategy.

These cases were identified using a research static analyzer for DMA
lifecycle ordering and subsequently reviewed against the driver
source. They are source-level lifetime concerns, not demonstrated
hardware exploits.

Regards,
Samarth Narang

Re: [BUG] serial: amba-pl011, imx, fsl_lpuart: ->flush_buffer() unmaps the TX DMA buffer before the transfer has stopped

[Greg Kroah-Hartman]

On Sat, Oct 31, 2026 at 12:54:25AM +0000, Samarth Narang wrote:
> I have compile-tested candidate patches for amba-pl011 and
> fsl_lpuart using this general approach. I can send them for review,
> or adapt them if the maintainers prefer a different synchronization
> strategy.

Please always just send patches, you do not need "permission" to do so.

thanks,

greg k-h

[PATCH 0/2] serial: keep the flushed TX buffer mapped until DMA has stopped

[Samarth Narang]

This series addresses the DMA buffer lifetime issues in amba-pl011 and
fsl_lpuart reported in the parent thread, where flush_buffer() unmaps
the TX buffer before the terminated DMA transfer is guaranteed to have
stopped accessing it. In fsl_lpuart, the buffer is unmapped even before
termination is requested.

Since flush_buffer() runs under the port lock and cannot wait for DMA
termination, both patches request termination while holding the lock and
defer unmapping to a work item that first calls dmaengine_synchronize().
The TX completion callback leaves the cleanup of flushed transfers to
this work item, while shutdown, and suspend in the case of fsl_lpuart,
waits for the work to finish before releasing the TX DMA state.

Both patches have only been compile-tested with arm64 defconfig, the
relevant drivers enabled, and W=1, with no new warnings. I do not have
access to PL011 or LPUART hardware with DMA support, so testing by
someone with suitable hardware would be appreciated.

The imx driver also needs a fix for the issue reported in the same
thread, but requires a different approach because its TX completion
callback unmaps the scatterlist unconditionally. I will send that patch
separately.

Samarth Narang (2):
  serial: amba-pl011: keep the flushed TX buffer mapped until DMA stops
  tty: serial: fsl_lpuart: don't unmap TX buffer before DMA has stopped

 drivers/tty/serial/amba-pl011.c | 54 +++++++++++++++++++++++++++++++--
 drivers/tty/serial/fsl_lpuart.c | 53 ++++++++++++++++++++++++++++----
 2 files changed, 98 insertions(+), 9 deletions(-)


base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
-- 
2.34.1

[PATCH 1/2] serial: amba-pl011: keep the flushed TX buffer mapped until DMA stops

[Samarth Narang]

pl011_dma_flush_buffer() is called with the port lock held and currently
unmaps the TX buffer immediately after requesting termination with
dmaengine_terminate_async(). Since asynchronous termination does not
guarantee that the transfer or its completion callback has finished, the
DMA controller may continue reading from the buffer after its mapping
has been released. Commit f6a196477184 ("serial: pl011: Fix DMA
->flush_buffer()") introduced asynchronous termination under the port
lock, but did not add the required synchronization before unmapping.

On systems using an IOMMU or SWIOTLB, outstanding DMA reads may access
an IOVA or bounce buffer that has already been reused, potentially
transmitting data from another mapping. On systems without either, the
outstanding transfer may instead race with the next refill of dmatx.buf.
This can be triggered by flushing the TTY while TX DMA is active.

Since flush_buffer() cannot sleep, defer the unmap to a work item that
calls dmaengine_synchronize() before releasing the mapping. Keep
dmatx.queued set to prevent buffer reuse, disable TX DMA requests, and
let the work item restart transmission if data remains pending. The
completion callback leaves flushed transfers for deferred cleanup, while
shutdown waits for the work to finish before tearing down TX DMA.

Fixes: f6a196477184 ("serial: pl011: Fix DMA ->flush_buffer()")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
---
 drivers/tty/serial/amba-pl011.c | 54 +++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index e6f652905ca6..863a050cf654 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -41,6 +41,7 @@
 #include <linux/sizes.h>
 #include <linux/io.h>
 #include <linux/acpi.h>
+#include <linux/workqueue.h>
 
 #define UART_NR			14
 
@@ -302,6 +303,8 @@ struct pl011_dmatx_data {
 	size_t			len;
 	char			*buf;
 	bool			queued;
+	bool			flushing;
+	struct work_struct	flush_work;
 };
 
 enum pl011_rs485_tx_state {
@@ -609,6 +612,12 @@ static void pl011_dma_tx_callback(void *data)
 	u16 dmacr;
 
 	uart_port_lock_irqsave(&uap->port, &flags);
+	if (uap->dmatx.flushing) {
+		/* pl011_dma_tx_flush_work() retires the flushed buffer */
+		uart_port_unlock_irqrestore(&uap->port, flags);
+		return;
+	}
+
 	if (uap->dmatx.queued)
 		dma_unmap_single(dmaengine_get_dma_device(dmatx->chan), dmatx->dma,
 				 dmatx->len, DMA_TO_DEVICE);
@@ -867,13 +876,47 @@ __acquires(&uap->port.lock)
 
 	dmaengine_terminate_async(uap->dmatx.chan);
 
-	if (uap->dmatx.queued) {
-		dma_unmap_single(dmaengine_get_dma_device(uap->dmatx.chan), uap->dmatx.dma,
-				 uap->dmatx.len, DMA_TO_DEVICE);
+	/*
+	 * The terminated transfer may still be reading dmatx.buf: keep it
+	 * mapped, and queued set so that nothing refills it, until
+	 * pl011_dma_tx_flush_work() has synchronized the channel.
+	 */
+	if (uap->dmatx.queued && !uap->dmatx.flushing) {
+		uap->dmatx.flushing = true;
+		uap->dmacr &= ~UART011_TXDMAE;
+		pl011_write(uap->dmacr, uap, REG_DMACR);
+		schedule_work(&uap->dmatx.flush_work);
+	}
+}
+
+/*
+ * flush_buffer runs under the port lock and can only request termination.
+ * Unmap the flushed TX buffer here, once the terminated transfer and its
+ * completion callback are known to have finished, then restart TX.
+ */
+static void pl011_dma_tx_flush_work(struct work_struct *work)
+{
+	struct uart_amba_port *uap =
+	    container_of(work, struct uart_amba_port, dmatx.flush_work);
+	struct tty_port *tport = &uap->port.state->port;
+	unsigned long flags;
+
+	dmaengine_synchronize(uap->dmatx.chan);
+
+	uart_port_lock_irqsave(&uap->port, &flags);
+	if (uap->dmatx.flushing) {
+		dma_unmap_single(dmaengine_get_dma_device(uap->dmatx.chan),
+				 uap->dmatx.dma, uap->dmatx.len, DMA_TO_DEVICE);
+		uap->dmatx.flushing = false;
 		uap->dmatx.queued = false;
 		uap->dmacr &= ~UART011_TXDMAE;
 		pl011_write(uap->dmacr, uap, REG_DMACR);
+		if (!uart_tx_stopped(&uap->port) &&
+		    !kfifo_is_empty(&tport->xmit_fifo) &&
+		    pl011_dma_tx_refill(uap) <= 0)
+			pl011_start_tx_pio(uap);
 	}
+	uart_port_unlock_irqrestore(&uap->port, flags);
 }
 
 static void pl011_dma_rx_callback(void *data);
@@ -1177,6 +1220,8 @@ static void pl011_dma_startup(struct uart_amba_port *uap)
 
 	/* The DMA buffer is now the FIFO the TTY subsystem can use */
 	uap->port.fifosize = PL011_DMA_BUFFER_SIZE;
+	uap->dmatx.flushing = false;
+	INIT_WORK(&uap->dmatx.flush_work, pl011_dma_tx_flush_work);
 	uap->using_tx_dma = true;
 
 	if (!uap->dmarx.chan)
@@ -1236,6 +1281,9 @@ static void pl011_dma_shutdown(struct uart_amba_port *uap)
 	if (!(uap->using_tx_dma || uap->using_rx_dma))
 		return;
 
+	if (uap->using_tx_dma)
+		flush_work(&uap->dmatx.flush_work);
+
 	/* Disable RX and TX DMA */
 	while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
 		cpu_relax();
-- 
2.34.1

[PATCH 2/2] tty: serial: fsl_lpuart: don't unmap TX buffer before DMA has stopped

[Samarth Narang]

lpuart_flush_buffer() unmaps the TX scatterlist before calling
dmaengine_terminate_async(), allowing eDMA to continue reading from the
buffer after its mapping has been released. Reversing these operations
would not be sufficient, since asynchronous termination does not
guarantee that DMA has stopped and dmaengine_synchronize() is required
before releasing memory accessed by the descriptor.

The issue can be triggered through TCFLSH(TCOFLUSH), hangup, or close
while TX DMA is active. On systems using SWIOTLB, outstanding DMA reads
may access a bounce slot that has already been reassigned to another
device, potentially transmitting its contents over the serial line.

Since flush_buffer() runs under the port lock and cannot sleep, request
termination there while keeping the scatterlist mapped and
dma_tx_in_progress set to prevent reuse. Defer unmapping to a work item
that first calls dmaengine_synchronize(), have the TX completion
callback ignore flushed transfers, and ensure that shutdown and suspend
wait for the work to finish before resetting the TX DMA state.

Fixes: 6250cc30c4c4 ("tty: serial: fsl_lpuart: Use scatter/gather DMA for Tx")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
---
 drivers/tty/serial/fsl_lpuart.c | 53 +++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c8575c965203..8343351199eb 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -26,6 +26,7 @@
 #include <linux/serial_core.h>
 #include <linux/slab.h>
 #include <linux/tty_flip.h>
+#include <linux/workqueue.h>
 
 /* All registers are 8-bit width */
 #define UARTBDH			0x00
@@ -280,6 +281,8 @@ struct lpuart_port {
 	unsigned int		dma_tx_bytes;
 	unsigned int		dma_rx_bytes;
 	bool			dma_tx_in_progress;
+	bool			dma_tx_flushing;
+	struct work_struct	dma_tx_flush_work;
 	unsigned int		dma_rx_timeout;
 	struct timer_list	lpuart_timer;
 	struct scatterlist	rx_sgl, tx_sgl[2];
@@ -530,7 +533,7 @@ static void lpuart_dma_tx_complete(void *arg)
 	unsigned long flags;
 
 	uart_port_lock_irqsave(&sport->port, &flags);
-	if (!sport->dma_tx_in_progress) {
+	if (!sport->dma_tx_in_progress || sport->dma_tx_flushing) {
 		uart_port_unlock_irqrestore(&sport->port, flags);
 		return;
 	}
@@ -558,6 +561,36 @@ static void lpuart_dma_tx_complete(void *arg)
 	uart_port_unlock_irqrestore(&sport->port, flags);
 }
 
+/*
+ * flush_buffer runs under the port lock and can only request termination.
+ * Unmap the TX scatterlist here, once the terminated transfer and its
+ * completion callback are known to have finished.
+ */
+static void lpuart_dma_tx_flush_work(struct work_struct *work)
+{
+	struct lpuart_port *sport = container_of(work, struct lpuart_port,
+						 dma_tx_flush_work);
+	struct dma_chan *chan = sport->dma_tx_chan;
+	unsigned long flags;
+
+	dmaengine_synchronize(chan);
+
+	uart_port_lock_irqsave(&sport->port, &flags);
+	if (sport->dma_tx_flushing) {
+		dma_unmap_sg(dmaengine_get_dma_device(chan), &sport->tx_sgl[0],
+			     sport->dma_tx_nents, DMA_TO_DEVICE);
+		sport->dma_tx_flushing = false;
+		sport->dma_tx_in_progress = false;
+	}
+	/* As in the TX callback: no restart while shutdown waits on dma_wait */
+	if (!waitqueue_active(&sport->dma_wait) &&
+	    !lpuart_stopped_or_empty(&sport->port))
+		lpuart_dma_tx(sport);
+	uart_port_unlock_irqrestore(&sport->port, flags);
+
+	wake_up(&sport->dma_wait);
+}
+
 static dma_addr_t lpuart_dma_datareg_addr(struct lpuart_port *sport)
 {
 	switch (sport->port.iotype) {
@@ -605,12 +638,16 @@ static void lpuart_flush_buffer(struct uart_port *port)
 	u32 fifo;
 
 	if (sport->lpuart_dma_tx_use) {
-		if (sport->dma_tx_in_progress) {
-			dma_unmap_sg(dmaengine_get_dma_device(chan), &sport->tx_sgl[0],
-				sport->dma_tx_nents, DMA_TO_DEVICE);
-			sport->dma_tx_in_progress = false;
-		}
 		dmaengine_terminate_async(chan);
+		/*
+		 * The transfer may still be reading the TX scatterlist: keep it
+		 * mapped (and dma_tx_in_progress set) until the flush work has
+		 * synchronized the channel.
+		 */
+		if (sport->dma_tx_in_progress && !sport->dma_tx_flushing) {
+			sport->dma_tx_flushing = true;
+			schedule_work(&sport->dma_tx_flush_work);
+		}
 	}
 
 	if (lpuart_is_32(sport)) {
@@ -1760,6 +1797,8 @@ static void lpuart_tx_dma_startup(struct lpuart_port *sport)
 		goto err;
 
 	init_waitqueue_head(&sport->dma_wait);
+	INIT_WORK(&sport->dma_tx_flush_work, lpuart_dma_tx_flush_work);
+	sport->dma_tx_flushing = false;
 	sport->lpuart_dma_tx_use = true;
 	if (lpuart_is_32(sport)) {
 		uartbaud = lpuart32_read(&sport->port, UARTBAUD);
@@ -1926,6 +1965,7 @@ static void lpuart_dma_shutdown(struct lpuart_port *sport)
 	}
 
 	if (sport->lpuart_dma_tx_use) {
+		flush_work(&sport->dma_tx_flush_work);
 		if (wait_event_interruptible_timeout(sport->dma_wait,
 			!sport->dma_tx_in_progress, msecs_to_jiffies(300)) <= 0) {
 			sport->dma_tx_in_progress = false;
@@ -3179,6 +3219,7 @@ static int lpuart_suspend(struct device *dev)
 				writeb(temp, sport->port.membase + UARTCR5);
 			}
 			uart_port_unlock_irqrestore(&sport->port, flags);
+			flush_work(&sport->dma_tx_flush_work);
 			sport->dma_tx_in_progress = false;
 			dmaengine_terminate_sync(sport->dma_tx_chan);
 		}
-- 
2.34.1

Re: [PATCH 0/2] serial: keep the flushed TX buffer mapped until DMA has stopped

[Greg Kroah-Hartman]

On Sat, Oct 31, 2026 at 12:37:47PM +0000, Samarth Narang wrote:
> This series addresses the DMA buffer lifetime issues in amba-pl011 and
> fsl_lpuart reported in the parent thread, where flush_buffer() unmaps
> the TX buffer before the terminated DMA transfer is guaranteed to have
> stopped accessing it. In fsl_lpuart, the buffer is unmapped even before
> termination is requested.
> 
> Since flush_buffer() runs under the port lock and cannot wait for DMA
> termination, both patches request termination while holding the lock and
> defer unmapping to a work item that first calls dmaengine_synchronize().
> The TX completion callback leaves the cleanup of flushed transfers to
> this work item, while shutdown, and suspend in the case of fsl_lpuart,
> waits for the work to finish before releasing the TX DMA state.
> 
> Both patches have only been compile-tested with arm64 defconfig, the
> relevant drivers enabled, and W=1, with no new warnings. I do not have
> access to PL011 or LPUART hardware with DMA support, so testing by
> someone with suitable hardware would be appreciated.
> 
> The imx driver also needs a fix for the issue reported in the same
> thread, but requires a different approach because its TX completion
> callback unmaps the scatterlist unconditionally. I will send that patch
> separately.

Again, these need to be tested on real hardware.  Not to mention the
bugs that sashiko just found in the changes as well.

thanks,

greg k-h

Re: [PATCH 0/2] serial: keep the flushed TX buffer mapped until DMA has stopped

[Samarth Narang]

Thanks Greg, I will address the findings Sashiko found, would
appreciate someone with hardware giving a stamp of approval!

On Sat, Oct 31, 2026 at 9:00 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Oct 31, 2026 at 12:37:47PM +0000, Samarth Narang wrote:
> > This series addresses the DMA buffer lifetime issues in amba-pl011 and
> > fsl_lpuart reported in the parent thread, where flush_buffer() unmaps
> > the TX buffer before the terminated DMA transfer is guaranteed to have
> > stopped accessing it. In fsl_lpuart, the buffer is unmapped even before
> > termination is requested.
> >
> > Since flush_buffer() runs under the port lock and cannot wait for DMA
> > termination, both patches request termination while holding the lock and
> > defer unmapping to a work item that first calls dmaengine_synchronize().
> > The TX completion callback leaves the cleanup of flushed transfers to
> > this work item, while shutdown, and suspend in the case of fsl_lpuart,
> > waits for the work to finish before releasing the TX DMA state.
> >
> > Both patches have only been compile-tested with arm64 defconfig, the
> > relevant drivers enabled, and W=1, with no new warnings. I do not have
> > access to PL011 or LPUART hardware with DMA support, so testing by
> > someone with suitable hardware would be appreciated.
> >
> > The imx driver also needs a fix for the issue reported in the same
> > thread, but requires a different approach because its TX completion
> > callback unmaps the scatterlist unconditionally. I will send that patch
> > separately.
>
> Again, these need to be tested on real hardware.  Not to mention the
> bugs that sashiko just found in the changes as well.
>
> thanks,
>
> greg k-h