94xhn opened a new pull request, #19391: URL: https://github.com/apache/nuttx/pull/19391
## Summary Fixes #14845. `uart_putxmitchar()` manipulates `dev->xmit.head`/`dev->xmit.buffer` directly with no locking of its own — every other caller takes `dev->xmit.lock` first (`uart_write()` locks once around its whole per-character write loop). `uart_readv()`'s `ECHO` handling calls `uart_putxmitchar()` in two places (the backspace/delete erase sequence, and normal character echo) without ever taking that lock, so a concurrent `uart_write()` and a local echo triggered by `uart_readv()` can race on the same circular transmit buffer state, corrupting it. ## Fix Take `dev->xmit.lock` around each echo's `uart_putxmitchar()` call(s), matching what `uart_write()` already does elsewhere in this same file. `uart_readv()` holds `dev->recv.lock` for the duration of its own read loop, so this introduces `recv.lock` → `xmit.lock` nesting. I checked every other use of `recv.lock`/`xmit.lock` in this file and no other code path ever acquires `recv.lock` while already holding `xmit.lock` (nothing outside `uart_readv()` touches `recv.lock` at all), so this doesn't create a new lock-ordering cycle/deadlock risk. ## Testing I don't have real serial hardware or a working local NuttX `sim` build environment (WSL crashed on this machine and I wasn't able to get it running again), so I can't provide a build/run log or a genuine concurrent-echo-vs-write repro. What I did instead: read every call site of `uart_putxmitchar()` and every use of `xmit.lock`/`recv.lock` in `drivers/serial/serial.c` to confirm (a) `uart_putxmitchar()` really has no internal synchronization, (b) `uart_write()`'s existing lock/unlock pattern is the one to mirror, and (c) the lock-ordering argument above. Happy to add a stronger repro or run `sim:nsh` if a maintainer can point me to (or confirm) a way to trigger it, e.g. via a two-thread test that concurrently `write()`s and triggers local echo on the same tty. ## Generative AI I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the code and the description above. -- 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]
