[PATCH] tty: serial: samsung: fix TX fifo underflow on flush race

[tianci xie]

uart_flush_buffer() resets the xmit kfifo and then calls ->flush_buffer()
with the port lock held.  Neither s3c64xx_serial_ops nor
apple_s5l_serial_ops implements that callback, and
s3c24xx_serial_tx_dma_complete() computes the transferred byte count from
dma->tx_bytes_requested before it takes the port lock.

A completion that has already sampled that count can therefore run after
a TCOFLUSH has reset the kfifo, and advance the freshly reset kfifo by
the byte count of the discarded transfer.  kfifo_skip_count() does not
clamp, so ->out moves past ->in and kfifo_len() wraps to a huge value.
The driver then keeps transmitting stale buffer contents, and a later
close can wait in tty_wait_until_sent() until it times out.

Handle this the way 8250 does in 9e512eaaf8f4 ("serial: 8250: Fix fifo
underflow on flush"):

 - add s3c24xx_serial_flush_buffer(), which clears the outstanding byte
   count and terminates the TX channel asynchronously.  The port lock is
   held here, so dmaengine_terminate_sync() cannot be used;

 - sample dma->tx_bytes_requested under the port lock, so that a
   completion racing with the flush observes the cleared value and
   advances by zero.

The callback may still run after dmaengine_terminate_async(); for the
fifo it is then a no-op, but it does clear tx_in_progress so that a new
transfer can be started.  As in 8250, this assumes that a terminated
descriptor is retired before the next one is queued.

Reported-by: Samarth Narang <samarth.colleges at gmail.com>
Link: https://lore.kernel.org/all/20261031004733.3345841-1-samarth.colleges@gmail.com/
Fixes: 29bef7990845 ("serial: samsung: add DMA support for TX")
Signed-off-by: tianci xie <xiex16070 at gmail.com>
---
 drivers/tty/serial/samsung_tty.c | 40 +++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 640f85e..e955b7e 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -330,7 +330,6 @@ static void s3c24xx_serial_tx_dma_complete(void *args)
 	int count;
 
 	dmaengine_tx_status(dma->tx_chan, dma->tx_cookie, &state);
-	count = dma->tx_bytes_requested - state.residue;
 	async_tx_ack(dma->tx_desc);
 
 	dma_sync_single_for_cpu(dmaengine_get_dma_device(dma->tx_chan),
@@ -339,6 +338,17 @@ static void s3c24xx_serial_tx_dma_complete(void *args)
 
 	uart_port_lock_irqsave(port, &flags);
 
+	/*
+	 * uart_flush_buffer() resets the xmit fifo and then calls
+	 * ->flush_buffer(), which clears tx_bytes_requested.  Sample the byte
+	 * count under the port lock so that a completion which raced with a
+	 * flush cannot advance the already reset fifo past its write index.
+	 */
+	if (dma->tx_bytes_requested)
+		count = dma->tx_bytes_requested - state.residue;
+	else
+		count = 0;
+
 	uart_xmit_advance(port, count);
 	ourport->tx_in_progress = 0;
 
@@ -1162,6 +1172,32 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
 	}
 }
 
+/*
+ * Called from uart_flush_buffer() with the port lock held, after the xmit fifo
+ * has already been reset.  A TX DMA transfer still in flight carries data that
+ * no longer exists, so drop its byte count before terminating it: the
+ * completion callback may still run and must not advance the reset fifo.
+ */
+static void s3c24xx_serial_flush_buffer(struct uart_port *port)
+{
+	struct s3c24xx_uart_port *ourport = to_ourport(port);
+	struct s3c24xx_uart_dma *dma = ourport->dma;
+
+	if (!dma || !dma->tx_chan || ourport->tx_in_progress != S3C24XX_TX_DMA)
+		return;
+
+	dma->tx_bytes_requested = 0;
+
+	/*
+	 * dmaengine_terminate_sync() cannot be used here because the port
+	 * lock is held.  The callback may or may not run afterwards; either
+	 * way it is a no-op for the fifo because tx_bytes_requested is zero.
+	 */
+	dmaengine_terminate_async(dma->tx_chan);
+
+	ourport->tx_in_progress = 0;
+}
+
 static void s3c64xx_serial_shutdown(struct uart_port *port)
 {
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
@@ -1684,6 +1720,7 @@ static const struct uart_ops s3c64xx_serial_ops = {
 	.stop_tx	= s3c24xx_serial_stop_tx,
 	.start_tx	= s3c24xx_serial_start_tx,
 	.stop_rx	= s3c24xx_serial_stop_rx,
+	.flush_buffer	= s3c24xx_serial_flush_buffer,
 	.break_ctl	= s3c24xx_serial_break_ctl,
 	.startup	= s3c64xx_serial_startup,
 	.shutdown	= s3c64xx_serial_shutdown,
@@ -1705,6 +1742,7 @@ static const struct uart_ops apple_s5l_serial_ops = {
 	.stop_tx	= s3c24xx_serial_stop_tx,
 	.start_tx	= s3c24xx_serial_start_tx,
 	.stop_rx	= s3c24xx_serial_stop_rx,
+	.flush_buffer	= s3c24xx_serial_flush_buffer,
 	.break_ctl	= s3c24xx_serial_break_ctl,
 	.startup	= apple_s5l_serial_startup,
 	.shutdown	= apple_s5l_serial_shutdown,