As Ilya Zykov identified in his patch 'PROBLEM: Race condition in
tty buffer's function flush_to_ldisc()', a race condition exists
which allows a parallel flush_to_ldisc() to flush and free the tty
flip buffers while those buffers are in-use. For example,
CPU 0 | CPU 1 | CPU 2
| flush_to_ldisc() |
| grab spin lock |
tty_buffer_flush() | | flush_to_ldisc()
wait for spin lock | | wait for spin lock
| if (!test_and_set_bit(TTYP_FLUSHING)) |
| while (next flip buffer) |
| ... |
| drop spin lock |
grab spin lock | |
if (test_bit(TTYP_FLUSHING)) | |
set_bit(TTYP_FLUSHPENDING) | receive_buf() |
drop spin lock | |
| | grab spin lock
| | if (!test_and_set_bit(TTYP_FLUSHING))
| | if (test_bit(TTYP_FLUSHPENDING))
| | __tty_buffer_flush()
CPU 2 has just flushed and freed all tty flip buffers while CPU 1 is
transferring data from the head flip buffer.
The original patch was rejected under the assumption that parallel
flush_to_ldisc() was not possible. Because of necessary changes to
the workqueue api, work items can execute in parallel on SMP.
This patch differs slightly from the original patch by testing for
a pending flush _after_ each receive_buf(), since TTYP_FLUSHPENDING
can only be set while the lock is dropped around receive_buf().
Reported-by: Ilya Zykov <linux@xxxxxxx>
Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
---
drivers/tty/tty_buffer.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index bb11993..9fd8acd 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -449,11 +449,6 @@ static void flush_to_ldisc(struct work_struct *work)
tty_buffer_free(port, head);
continue;
}
- /* Ldisc or user is trying to flush the buffers
- we are feeding to the ldisc, stop feeding the
- line discipline as we want to empty the queue */
- if (test_bit(TTYP_FLUSHPENDING, &port->iflags))
- break;
if (!tty->receive_room)
break;
if (count > tty->receive_room)
@@ -465,17 +460,20 @@ static void flush_to_ldisc(struct work_struct *work)
disc->ops->receive_buf(tty, char_buf,
flag_buf, count);
spin_lock_irqsave(&buf->lock, flags);
+ /* Ldisc or user is trying to flush the buffers.
+ We may have a deferred request to flush the
+ input buffer, if so pull the chain under the lock
+ and empty the queue */
+ if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) {
+ __tty_buffer_flush(port);
+ clear_bit(TTYP_FLUSHPENDING, &port->iflags);
+ wake_up(&tty->read_wait);
+ break;
+ }
}
clear_bit(TTYP_FLUSHING, &port->iflags);
}
- /* We may have a deferred request to flush the input buffer,
- if so pull the chain under the lock and empty the queue */
- if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) {
- __tty_buffer_flush(port);
- clear_bit(TTYP_FLUSHPENDING, &port->iflags);
- wake_up(&tty->read_wait);
- }
spin_unlock_irqrestore(&buf->lock, flags);
tty_ldisc_deref(disc);
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
