apache
{bp-16466} drivers/serial: fix race conditions
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
