This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch releases/12.10 in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit ea93ddfc08787b500fcd77b01f72aac7a502be99 Author: Kerogit <[email protected]> AuthorDate: Sun Jun 8 23:08:55 2025 +0200 drivers/serial/serial: fix race condition in flow control This patch fixes calculation of nbuffered value if CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS is set. Volatile variable that can be changed in interrupt handler was used in a condition which branched the calculation into two paths. Precisely timed interrupt could make the branch that was taken the incorrect one. Patch was tested by building on AVR DA/DB chip. Signed-off-by: Kerogit <[email protected]> --- drivers/serial/serial.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 6796eb3df9..9f25b2a04c 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -901,6 +901,7 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio) #ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS unsigned int nbuffered; unsigned int watermark; + int16_t head; #endif irqstate_t flags; ssize_t recvd = 0; @@ -1337,16 +1338,23 @@ static ssize_t uart_readv(FAR struct file *filep, FAR struct uio *uio) #ifdef CONFIG_SERIAL_IFLOWCONTROL #ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS - /* How many bytes are now buffered */ + /* How many bytes are now buffered. Head needs to be copied + * to a non-volatile variable to prevent TOCTOU error in case + * the interrupt handler changes it between comparison and assignment. + * (Copy of tail is not strictly needed but saves us few instructions.) + */ rxbuf = &dev->recv; - if (rxbuf->head >= rxbuf->tail) + head = rxbuf->head; + tail = rxbuf->tail; + + if (head >= tail) { - nbuffered = rxbuf->head - rxbuf->tail; + nbuffered = head - tail; } else { - nbuffered = rxbuf->size - rxbuf->tail + rxbuf->head; + nbuffered = rxbuf->size - tail + head; } /* Is the level now below the watermark level that we need to report? */
