Aksal/nuttx
3
1
Fork 1
mirror of https://github.com/apache/nuttx.git synced 2026-09-11 03:16:45 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

drivers/serial: read the consumer index once in uart_recvchars()

Browse source 
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 <raiden00@railab.me>
This commit is contained in:
raiden00pl 2026-09-08 09:40:31 +02:00 • committed by Xiang Xiao
parent a6393fa320
commit a3d4802ff8
1 changed files with 15 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

22
drivers/serial/serial_io.c
View file

@ -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;
}