Race condition in tty_flip_buffer_push/flush_to_ldisc?
I think I've stumbled over a race condition in drivers/char/tty_io.c, tty_flip_buffer_push()/flush_to_ldisc(). I noticed it in 2.4.19, but the code in 2.5.49 looks similar.
Suppose I'm running a serial port with low_latency enabled. The low-level serial interrupt handler will call tty_flip_buffer_push() which will immediately call flush_to_ldisc() due to low_latency being set. In flush_to_ldisc(), if the TTY_DONT_FLIP bit is set, it will add itself to the timer task queue and return. When the task queue gets processed, processing might get interrupted by another serial interrupt or vice versa, resulting in 2 concurrent calls to flush_to_ldisc(). This time, the TTY_DONT_FLIP bit probably isn't set, so they both will try to process the same flip buffer.
Note that reading the current flip buffer pointers isn't protected by cli(), only modifying them is. And even if it were, we could end up calling tty->ldisc.receive_buf() twice for the same tty which probably isn't safe either.
Any ideas on how to fix this?
And before you ask - yes, I do need low_latency. Not because of the low latecy, but because I'm trying to use 921600 bps. At this speed, a 512 Bytes flip buffer gets filled in 5.5ms while timer task queue flip buffer processing only flushes it every 10ms, resulting in slighly more characters lost than I can accept :-)
Thanks,
Martin
From: "Martin Buck" <[email protected]>
Quoted 6 lines
Suppose I'm running a serial port with
low_latencyenabled. The low-level serial interrupt handler will calltty_flip_buffer_push()which will immediately callflush_to_ldisc()due tolow_latencybeing set. Inflush_to_ldisc(), if theTTY_DONT_FLIPbit is set, it will add itself to the timer task queue and return. When the task queue gets processed, processing might get interrupted by another serial interrupt or vice
versa,
Quoted 3 lines
resulting in 2 concurrent calls to
flush_to_ldisc(). This time, theTTY_DONT_FLIPbit probably isn't set, so they both will try to process the same flip buffer.
What causes the DONT_FLIP bit to be un/set? I don't think your situation can occur under normal operation. But ICBW.
Quoted 2 lines
Note that reading the current flip buffer pointers isn't protected by
cli(), only modifying them is. And even if it were, we could end up
calling
Quoted 4 lines
tty->ldisc.receive_buf()twice for the same tty which probably isn't safe either.Any ideas on how to fix this?
If you search the lkml archive, recently Ted stated that it's acceptable to call the line discpline's receive_buf() routine directly, bypassing the flip buffers.
..Stu
Quoted 2 lines · Stuart MacDonald
What causes the
DONT_FLIPbit to be un/set? I don't think your situation can occur under normal operation. But ICBW.
n_tty.c seems to do it in read_chan(), i.e. it will happen when the process reading from the TTY is currently in read(2) while at the same time new characters arrive on the serial port.
Quoted 3 lines
If you search the lkml archive, recently Ted stated that it's acceptable to call the line discpline's
receive_buf()routine directly, bypassing the flip buffers.
Interesting, that should solve my problem. But then the next question is:
What's the use of the DONT_FLIP bit if it's legal to bypass it by calling receive_buf() directly?
Also, if somebody enables low_latency using setserial with the standard serial driver, he can still trigger the race condition. It's not very likely to happen, but it does happen - I added code to flush_to_ldisc() to detect concurrent calls and it triggered once during an over-night test.
Thanks,
Martin