linux-kernel - [PATCH v2 1/2] tty: serial: 8250

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250506150748.3162-1-mans@mansr.com>
Date: Tue,  6 May 2025 16:07:29 +0100
From: Mans Rullgard <mans@...sr.com>
To: jirislaby@...nel.org
Cc: gregkh@...uxfoundation.org,
	linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org,
	linux-omap@...r.kernel.org,
	stable@...r.kernel.org
Subject: [PATCH v2 1/2] tty: serial: 8250_omap: fix tx with dma

Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
introduced an error in the TX DMA handling for 8250_omap.

When the OMAP_DMA_TX_KICK flag is set, one byte is pulled from the
kfifo and emitted directly in order to start the DMA.  This is done
without updating DMA tx_size which leads to uart_xmit_advance() called
in the DMA complete callback advancing the kfifo by one too much.

In practice, transmitting N bytes has been seen to result in the last
N-1 bytes being sent repeatedly.

This change fixes the problem by moving all of the dma setup after
the OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the
dma size for the 4-byte cutoff check. This slightly changes the
behaviour at buffer wraparound, but it still transmits the correct
bytes somehow. At the point kfifo_dma_out_prepare_mapped is called,
at least one byte is guaranteed to be in the fifo, so checking the
return value is not necessary.

Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
Cc: stable@...r.kernel.org
Signed-off-by: Mans Rullgard <mans@...sr.com>
---
v2: split patch in two
---
 drivers/tty/serial/8250/8250_omap.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index f1aee915bc02..180466e09605 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1173,16 +1173,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 		return 0;
 	}
 
-	sg_init_table(&sg, 1);
-	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-					   UART_XMIT_SIZE, dma->tx_addr);
-	if (ret != 1) {
-		serial8250_clear_THRI(p);
-		return 0;
-	}
-
-	dma->tx_size = sg_dma_len(&sg);
-
 	if (priv->habit & OMAP_DMA_TX_KICK) {
 		unsigned char c;
 		u8 tx_lvl;
@@ -1207,7 +1197,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 			ret = -EBUSY;
 			goto err;
 		}
-		if (dma->tx_size < 4) {
+		if (kfifo_len(&tport->xmit_fifo) < 4) {
 			ret = -EINVAL;
 			goto err;
 		}
@@ -1216,11 +1206,12 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 			goto err;
 		}
 		skip_byte = c;
-		/* now we need to recompute due to kfifo_get */
-		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-				UART_XMIT_SIZE, dma->tx_addr);
 	}
 
+	sg_init_table(&sg, 1);
+	kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
+				     UART_XMIT_SIZE, dma->tx_addr);
+
 	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
@@ -1228,6 +1219,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 		goto err;
 	}
 
+	dma->tx_size = sg_dma_len(&sg);
 	dma->tx_running = 1;
 
 	desc->callback = omap_8250_dma_tx_complete;
@@ -1248,8 +1240,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 err:
 	dma->tx_err = 1;
 out_skip:
-	if (skip_byte >= 0)
+	if (skip_byte >= 0) {
 		serial_out(p, UART_TX, skip_byte);
+		p->port.icount.tx++;
+	}
 	return ret;
 }
 
-- 
2.49.0