diff options
| author | Peter Hurley <peter@hurleysoftware.com> | 2013-03-20 13:20:43 -0400 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2013-04-09 17:05:02 -0700 |
| commit | 39f610e40eecc39e4c34e047fd6904ca6b525520 (patch) | |
| tree | 39af27120456a3a3b68b7068d65a71f5614db008 /drivers/tty | |
| parent | 31bdfc649f4577c9b1120c67b7b1e85f7777aad0 (diff) | |
| download | linux-3.10-39f610e40eecc39e4c34e047fd6904ca6b525520.tar.gz linux-3.10-39f610e40eecc39e4c34e047fd6904ca6b525520.tar.bz2 linux-3.10-39f610e40eecc39e4c34e047fd6904ca6b525520.zip | |
tty: Fix race condition if flushing tty flip buffers
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@izyk.ru>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Ilya Zykov <linux@izyk.ru>
Cc: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/tty')
| -rw-r--r-- | drivers/tty/tty_buffer.c | 22 |
1 files changed, 10 insertions, 12 deletions
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c index 578aa7594b1..9121c1f7aee 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); |