Revert "tty: serial: simplify qcom_geni_serial_send_chunk_fifo()"

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>

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e63a8fbe63bdb22b70fd9362fae5d557c4c59b75..99e08737f293c6868e56d2de80f31bf0e3345ca3 100644 (file)
--- 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(struct uart_port *uport)
 }
 
 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);