diff options
Diffstat (limited to 'queue-6.7/revert-tty-serial-simplify-qcom_geni_serial_send_chunk_fifo.patch')
| -rw-r--r-- | queue-6.7/revert-tty-serial-simplify-qcom_geni_serial_send_chunk_fifo.patch | 92 |
1 files changed, 92 insertions, 0 deletions
diff --git a/queue-6.7/revert-tty-serial-simplify-qcom_geni_serial_send_chunk_fifo.patch b/queue-6.7/revert-tty-serial-simplify-qcom_geni_serial_send_chunk_fifo.patch new file mode 100644 index 0000000000..20ed6f190b --- /dev/null +++ b/queue-6.7/revert-tty-serial-simplify-qcom_geni_serial_send_chunk_fifo.patch @@ -0,0 +1,92 @@ +From 3d9319c27ceb35fa3d2c8b15508967f3fc7e5b78 Mon Sep 17 00:00:00 2001 +From: Douglas Anderson <dianders@chromium.org> +Date: Mon, 4 Mar 2024 17:49:53 -0800 +Subject: Revert "tty: serial: simplify qcom_geni_serial_send_chunk_fifo()" + +From: Douglas Anderson <dianders@chromium.org> + +commit 3d9319c27ceb35fa3d2c8b15508967f3fc7e5b78 upstream. + +This reverts commit 5c7e105cd156fc9adf5294a83623d7a40c15f9b9. + +As identified by KASAN, the simplification done by the cleanup patch +was not legal. + +>From tracing through the code, it can be seen that we're transmitting +from a 4096-byte circular buffer. We copy anywhere from 1-4 bytes from +it each time. The simplification runs into trouble when we get near +the end of the circular buffer. For instance, we might start out with +xmit->tail = 4094 and we want to transfer 4 bytes. With the code +before simplification this was no problem. We'd read buf[4094], +buf[4095], buf[0], and buf[1]. With the new code we'll do a +memcpy(&buf[4094], 4) which reads 2 bytes past the end of the buffer +and then skips transmitting what's at buf[0] and buf[1]. + +KASAN isn't 100% consistent at reporting this for me, but to be extra +confident in the analysis, I added traces of the tail and tx_bytes and +then wrote a test program: + + while true; do + echo -n "abcdefghijklmnopqrstuvwxyz0" > /dev/ttyMSM0 + sleep .1 + done + +I watched the traces over SSH and saw: + qcom_geni_serial_send_chunk_fifo: 4093 4 + qcom_geni_serial_send_chunk_fifo: 1 3 + +Which indicated that one byte should be missing. Sure enough the +output that should have been: + + abcdefghijklmnopqrstuvwxyz0 + +In one case was actually missing a byte: + + abcdefghijklmnopqrstuvwyz0 + +Running "ls -al" on large directories also made the missing bytes +obvious since columns didn't line up. + +While the original code may not be the most elegant, we only talking +about copying up to 4 bytes here. Let's just go back to the code that +worked. + +Fixes: 5c7e105cd156 ("tty: serial: simplify qcom_geni_serial_send_chunk_fifo()") +Cc: stable <stable@kernel.org> +Signed-off-by: Douglas Anderson <dianders@chromium.org> +Acked-by: Jiri Slaby <jirislaby@kernel.org> +Tested-by: Johan Hovold <johan+linaro@kernel.org> +Link: https://lore.kernel.org/r/20240304174952.1.I920a314049b345efd1f69d708e7f74d2213d0b49@changeid +Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> +--- + drivers/tty/serial/qcom_geni_serial.c | 10 ++++++---- + 1 file changed, 6 insertions(+), 4 deletions(-) + +--- a/drivers/tty/serial/qcom_geni_serial.c ++++ b/drivers/tty/serial/qcom_geni_serial.c +@@ -851,19 +851,21 @@ static void qcom_geni_serial_stop_tx(str + } + + static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport, +- unsigned int remaining) ++ unsigned int chunk) + { + struct qcom_geni_serial_port *port = to_dev_port(uport); + struct circ_buf *xmit = &uport->state->xmit; +- unsigned int tx_bytes; ++ unsigned int tx_bytes, c, remaining = chunk; + u8 buf[BYTES_PER_FIFO_WORD]; + + while (remaining) { + memset(buf, 0, sizeof(buf)); + tx_bytes = min(remaining, BYTES_PER_FIFO_WORD); + +- memcpy(buf, &xmit->buf[xmit->tail], tx_bytes); +- uart_xmit_advance(uport, tx_bytes); ++ for (c = 0; c < tx_bytes ; c++) { ++ buf[c] = xmit->buf[xmit->tail]; ++ uart_xmit_advance(uport, 1); ++ } + + iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1); + |