• Douglas Anderson's avatar
    Revert "tty: serial: simplify qcom_geni_serial_send_chunk_fifo()" · 3d9319c2
    Douglas Anderson authored
    This reverts commit 5c7e105c.
    
    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: 5c7e105c ("tty: serial: simplify qcom_geni_serial_send_chunk_fifo()")
    Cc: stable <stable@kernel.org>
    Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
    Acked-by: default avatarJiri Slaby <jirislaby@kernel.org>
    Tested-by: default avatarJohan Hovold <johan+linaro@kernel.org>
    Link: https://lore.kernel.org/r/20240304174952.1.I920a314049b345efd1f69d708e7f74d2213d0b49@changeidSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    3d9319c2
qcom_geni_serial.c 49.1 KB