From a6393fa3201f6f4cf1b86d72417aa84efe37983c Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 20:31:47 +0200 Subject: [PATCH] drivers/serial: sample xmit.head once per iteration in uart_xmitchars() 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 b319c27f03e ("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 45c38d8592b ("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 --- drivers/serial/serial_io.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/serial/serial_io.c b/drivers/serial/serial_io.c index 0f8bc6b2708..22ee4159017 100644 --- a/drivers/serial/serial_io.c +++ b/drivers/serial/serial_io.c @@ -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 {