aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDouglas Anderson <dianders@chromium.org>2024-03-04 17:49:53 -0800
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2024-03-05 13:40:34 +0000
commit3d9319c27ceb35fa3d2c8b15508967f3fc7e5b78 (patch)
tree0a9eba34eee9c50931975c3abe8a5527b723a186
parent74cb7e0355fae9641f825afa389d3fba3b617714 (diff)
downloadaudit-3d9319c27ceb35fa3d2c8b15508967f3fc7e5b78.tar.gz
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>
-rw-r--r--drivers/tty/serial/qcom_geni_serial.c10
1 files changed, 6 insertions, 4 deletions
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e63a8fbe63bdb2..99e08737f293c6 100644
--- 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);