[PATCH v1] serial: qcom_geni: fix kfifo underflow when flush precedes DMA completion IRQ

Viken Dadhaniya posted 1 patch 1 month, 1 week ago
drivers/tty/serial/qcom_geni_serial.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
[PATCH v1] serial: qcom_geni: fix kfifo underflow when flush precedes DMA completion IRQ
Posted by Viken Dadhaniya 1 month, 1 week ago
When uart_flush_buffer() runs before the DMA completion IRQ is delivered,
the following race can occur (all steps serialized by uart_port_lock):

  1. DMA starts: tx_remaining = N, kfifo contains N bytes
  2. DMA completes in hardware; IRQ is pending but not yet delivered
  3. uart_flush_buffer() acquires the port lock and calls kfifo_reset(),
     making kfifo_len() = 0 while tx_remaining remains N
  4. uart_flush_buffer() releases the port lock
  5. DMA IRQ fires; handle_tx_dma() acquires the port lock and calls
     uart_xmit_advance(uport, tx_remaining) on an empty kfifo

uart_xmit_advance() increments kfifo->out by tx_remaining. Since
kfifo_reset() already set both in and out to 0, out wraps past in,
causing kfifo_len() to return UART_XMIT_SIZE - tx_remaining. The next
start_tx_dma() call then submits a DMA transfer of stale buffer data.

Fix this by snapshotting kfifo_len() at the start of handle_tx_dma()
and skipping uart_xmit_advance() when fifo_len < tx_remaining, which
indicates the kfifo was reset by a preceding flush.

Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
Cc: stable@vger.kernel.org
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index b365dd5da3cb..3c1be7b21290 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1031,8 +1031,20 @@ static void qcom_geni_serial_handle_tx_dma(struct uart_port *uport)
 {
 	struct qcom_geni_serial_port *port = to_dev_port(uport);
 	struct tty_port *tport = &uport->state->port;
+	unsigned int fifo_len = kfifo_len(&tport->xmit_fifo);
+
+	/*
+	 * Only advance the kfifo if it still contains the bytes that were
+	 * transferred. uart_flush_buffer() may have run before this IRQ
+	 * fired: it calls kfifo_reset() under the port lock, making
+	 * fifo_len = 0 while tx_remaining remains non-zero. Calling
+	 * uart_xmit_advance() in that case would underflow kfifo->out past
+	 * kfifo->in, making kfifo_len() wrap to UART_XMIT_SIZE - tx_remaining
+	 * and triggering a spurious large DMA transfer of stale data.
+	 */
+	if (fifo_len >= port->tx_remaining)
+		uart_xmit_advance(uport, port->tx_remaining);
 
-	uart_xmit_advance(uport, port->tx_remaining);
 	geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr, port->tx_remaining);
 	port->tx_dma_addr = 0;
 	port->tx_remaining = 0;

---
base-commit: 4cd074ae20bbcc293bbbce9163abe99d68ae6ae0
change-id: 20260506-serial-dma-stale-tx-buf-f53db336a13a

Best regards,
--  
Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Re: [PATCH v1] serial: qcom_geni: fix kfifo underflow when flush precedes DMA completion IRQ
Posted by Bartosz Golaszewski 1 month, 1 week ago
On Wed, 6 May 2026 06:45:21 +0200, Viken Dadhaniya
<viken.dadhaniya@oss.qualcomm.com> said:
> When uart_flush_buffer() runs before the DMA completion IRQ is delivered,
> the following race can occur (all steps serialized by uart_port_lock):
>
>   1. DMA starts: tx_remaining = N, kfifo contains N bytes
>   2. DMA completes in hardware; IRQ is pending but not yet delivered
>   3. uart_flush_buffer() acquires the port lock and calls kfifo_reset(),
>      making kfifo_len() = 0 while tx_remaining remains N
>   4. uart_flush_buffer() releases the port lock
>   5. DMA IRQ fires; handle_tx_dma() acquires the port lock and calls
>      uart_xmit_advance(uport, tx_remaining) on an empty kfifo
>
> uart_xmit_advance() increments kfifo->out by tx_remaining. Since
> kfifo_reset() already set both in and out to 0, out wraps past in,
> causing kfifo_len() to return UART_XMIT_SIZE - tx_remaining. The next
> start_tx_dma() call then submits a DMA transfer of stale buffer data.
>
> Fix this by snapshotting kfifo_len() at the start of handle_tx_dma()
> and skipping uart_xmit_advance() when fifo_len < tx_remaining, which
> indicates the kfifo was reset by a preceding flush.
>
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Cc: stable@vger.kernel.org
> Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
> ---

Make sense, thanks.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>