Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Peter Hurley and committed by
Greg Kroah-Hartman
39f610e4 31bdfc64

+10 -12
+10 -12
drivers/tty/tty_buffer.c
··· 449 449 tty_buffer_free(port, head); 450 450 continue; 451 451 } 452 - /* Ldisc or user is trying to flush the buffers 453 - we are feeding to the ldisc, stop feeding the 454 - line discipline as we want to empty the queue */ 455 - if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) 456 - break; 457 452 if (!tty->receive_room) 458 453 break; 459 454 if (count > tty->receive_room) ··· 460 465 disc->ops->receive_buf(tty, char_buf, 461 466 flag_buf, count); 462 467 spin_lock_irqsave(&buf->lock, flags); 468 + /* Ldisc or user is trying to flush the buffers. 469 + We may have a deferred request to flush the 470 + input buffer, if so pull the chain under the lock 471 + and empty the queue */ 472 + if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) { 473 + __tty_buffer_flush(port); 474 + clear_bit(TTYP_FLUSHPENDING, &port->iflags); 475 + wake_up(&tty->read_wait); 476 + break; 477 + } 463 478 } 464 479 clear_bit(TTYP_FLUSHING, &port->iflags); 465 480 } 466 481 467 - /* We may have a deferred request to flush the input buffer, 468 - if so pull the chain under the lock and empty the queue */ 469 - if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) { 470 - __tty_buffer_flush(port); 471 - clear_bit(TTYP_FLUSHPENDING, &port->iflags); 472 - wake_up(&tty->read_wait); 473 - } 474 482 spin_unlock_irqrestore(&buf->lock, flags); 475 483 476 484 tty_ldisc_deref(disc);