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: sample xmit.head once per iteration in uart_xmitchars()

Browse source 
uart_putxmitchar() advances xmit.head from thread context without
holding the critical section, so on SMP the head index can move, and
wrap around, while uart_xmitchars() runs in the TX interrupt on another
CPU.  Since commit b319c27f03 ("serial: Added APIs for receiving and
sending multiple chars") the sendbuf path of uart_xmitchars() reads
xmit.head twice: once to decide whether the pending data is contiguous
and again to compute its length.  If the producer wraps the index in
between, the computed length goes negative, is passed to sendbuf() as a
huge size_t and the driver transmits memory far beyond the ring buffer.

The per-byte path reads the index only once and is not affected, which
is why this went unnoticed: the batch path is only used by drivers that
implement sendbuf, and the 16550 driver gained it in commit 45c38d8592
("drivers/serial/16550: add polling mode support for serial drivers").
qemu-intel64 with SMP is the first configuration combining a sendbuf
driver with a producer running on another CPU.

On qemu-intel64 SMP this shows up as an endless stream of NUL bytes on
the console (captured with gdb: head = 1, tail = 8, size = 16, and
u16550_sendbuf() called with size = (size_t)-7), which makes the ntfc
test harness fail every test that runs while the flood lasts.

Read the head index once per loop iteration and use that snapshot for
both the contiguity test and the length.  The producer only ever moves
the index forward, so a stale snapshot merely sends less now.

Assisted-by: Claude Code
Signed-off-by: raiden00pl <raiden00@railab.me>
This commit is contained in:
raiden00pl 2026-09-07 20:31:47 +02:00 • committed by Xiang Xiao
parent a3bc1f04a0
commit a6393fa320
1 changed files with 12 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

16
drivers/serial/serial_io.c
View file

@ -57,14 +57,22 @@
void uart_xmitchars(FAR uart_dev_t *dev)
{
uint16_t nbytes = 0;
sbuf_size_t head;
#ifdef CONFIG_SMP
irqstate_t flags = enter_critical_section();
#endif
/* Send while we still have data in the TX buffer & room in the fifo */
/* Send while we still have data in the TX buffer & room in the fifo.
*
* uart_putxmitchar() advances xmit.head 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: a stale value only
* makes us send less now, whereas reading it twice can turn the batch
* length negative and send from far beyond the buffer.
*/
while (dev->xmit.head != dev->xmit.tail && uart_txready(dev))
while ((head = dev->xmit.head) != dev->xmit.tail && uart_txready(dev))
{
/* Send the next byte */
@ -72,9 +80,9 @@ void uart_xmitchars(FAR uart_dev_t *dev)
{
ssize_t sent;
if (dev->xmit.tail < dev->xmit.head)
if (dev->xmit.tail < head)
{
sent = dev->xmit.head - dev->xmit.tail;
sent = head - dev->xmit.tail;
}
else
{