From a3d4802ff858f38ffa4a90cc76cf009d50204a12 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 09:40:31 +0200 Subject: [PATCH] drivers/serial: read the consumer index once in uart_recvchars() Same issue as the previous commit, on the receive side: uart_read() advances recv.tail from thread context without holding the critical section, but the recvbuf batch path of uart_recvchars() reads recv.tail several times (the full check, the watermark count and the free-space computation). If uart_read() moves and wraps the index in between, the computed free space goes negative and is passed to recvbuf() as a huge size_t, which lets the driver store past the end of the ring buffer. Read recv.tail once per loop iteration and derive everything from that snapshot. The consumer only ever moves the index forward, so a stale snapshot merely stores less now. Assisted-by: Claude Code Signed-off-by: raiden00pl --- drivers/serial/serial_io.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/serial/serial_io.c b/drivers/serial/serial_io.c index 22ee4159017..3361c11f3f4 100644 --- a/drivers/serial/serial_io.c +++ b/drivers/serial/serial_io.c @@ -172,8 +172,16 @@ void uart_recvchars(FAR uart_dev_t *dev) while (uart_rxavailable(dev)) { + /* uart_read() advances recv.tail from thread context without holding + * the critical section, so on SMP it can move (and wrap) while we are + * in here. Sample it once per iteration and derive the free space + * from that snapshot: a stale value only makes us store less now, + * whereas reading it twice can turn the batch length negative. + */ + int nexthead = rxbuf->head + 1 < rxbuf->size ? rxbuf->head + 1 : 0; - bool is_full = (nexthead == rxbuf->tail); + sbuf_size_t tail = rxbuf->tail; + bool is_full = (nexthead == tail); FAR char *pbuf = NULL; char ch; @@ -182,13 +190,13 @@ void uart_recvchars(FAR uart_dev_t *dev) /* How many bytes are buffered */ - if (rxbuf->head >= rxbuf->tail) + if (rxbuf->head >= tail) { - nbuffered = rxbuf->head - rxbuf->tail; + nbuffered = rxbuf->head - tail; } else { - nbuffered = rxbuf->size - rxbuf->tail + rxbuf->head; + nbuffered = rxbuf->size - tail + rxbuf->head; } /* Is the level now above the watermark level that we need to report? */ @@ -231,11 +239,11 @@ void uart_recvchars(FAR uart_dev_t *dev) if (!is_full) { - if (rxbuf->tail > rxbuf->head) + if (tail > rxbuf->head) { - nbytes = rxbuf->tail - rxbuf->head - 1; + nbytes = tail - rxbuf->head - 1; } - else if (rxbuf->tail) + else if (tail) { nbytes = rxbuf->size - rxbuf->head; }