jerpelea opened a new pull request, #16573: URL: https://github.com/apache/nuttx/pull/16573
## Summary While going through the code in drivers/serial/serial.c, I noticed this comment: The head and tail pointers are 16-bit values. The only time that the following could be unsafe is if the CPU made two non-atomic 8-bit accesses to obtain the 16-bit head index. This is what happens for (at least) AVR architecture. These are 8bit microcontrollers and as such, they will fetch the 16-bit value in two 8-bit load instructions; interrupt routine may execute between those and change the value being read. This will result in corrupted read (one byte of the value will be from pre-interrupt state and the other from post-interrupt state.) This patch introduces CONFIG_ARCH_LD_16BIT_NOT_ATOMIC configuration option, which is automatically selected for architectures known to be unable to load 16bit values in one instruction. (It is currently only set for AVR. I presume it might be also useful for Z80 but I do not have any experience with that architecture so I did no changes there.) When this configuration option is set, reads of recv.head and xmit.tail are enclosed with enter_critical_section and leave_critical_section calls to prevent interrupt handlers from running, if needed. Not all reads need to be protected this way - some are already in existing critical sections and some happen with the UART-specific interrupt disabled. If the configuration option is not set, the code simply loads the value into a local variable. Subsequent direct uses of the unprotected volatile variable are replaced with the local variables for both cases. There is also a related change that only applies when CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS is set. Aside from the protection selected by CONFIG_ARCH_LD_16BIT_NOT_ATOMIC, this patch also fixes calculation of the nbuffered value. This calculation is not running with interrupts disabled and value of rxbuf->head can change between the condition and actual computation. Even if the load itself is atomic, this leads to TOCTOU error. ## Impact RELEASE ## Testing CI -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
