Commit 5c58ceff authored by Linus Torvalds's avatar Linus Torvalds

tty: make sure to flush any pending work when halting the ldisc

When I rewrote tty ldisc code to use proper reference counts (commits
65b77046 and cbe9352f) in order to avoid a race with hangup, the
test-program that Eric Biederman used to trigger the original problem
seems to have exposed another long-standing bug: the hangup code did the
'tty_ldisc_halt()' to stop any buffer flushing activity, but unlike the
other call sites it never actually flushed any pending work.

As a result, if you get just the right timing, the pending work may be
just about to execute (ie the timer has already triggered and thus
cancel_delayed_work() was a no-op), when we then re-initialize the ldisc
from under it.

That, in turn, results in various random problems, usually seen as a
NULL pointer dereference in run_timer_softirq() or a BUG() in
worker_thread (but it can be almost anything).

Fix it by adding the required 'flush_scheduled_work()' after doing the
tty_ldisc_halt() (this also requires us to move the ldisc halt to before
taking the ldisc mutex in order to avoid a deadlock with the workqueue
executing do_tty_hangup, which requires the mutex).

The locking should be cleaned up one day (the requirement to do this
outside the ldisc_mutex is very annoying, and weakens the lock), but
that's a larger and separate undertaking.
Reported-by: default avatarEric W. Biederman <ebiederm@xmission.com>
Tested-by: default avatarXiaotian Feng <xtfeng@gmail.com>
Tested-by: default avatarYanmin Zhang <yanmin_zhang@linux.intel.com>
Tested-by: default avatarDave Young <hidave.darkstar@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 7111dc73
...@@ -508,8 +508,9 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old) ...@@ -508,8 +508,9 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
* be obtained while the delayed work queue halt ensures that no more * be obtained while the delayed work queue halt ensures that no more
* data is fed to the ldisc. * data is fed to the ldisc.
* *
* In order to wait for any existing references to complete see * You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex)
* tty_ldisc_wait_idle. * in order to make sure any currently executing ldisc work is also
* flushed.
*/ */
static int tty_ldisc_halt(struct tty_struct *tty) static int tty_ldisc_halt(struct tty_struct *tty)
...@@ -753,11 +754,14 @@ void tty_ldisc_hangup(struct tty_struct *tty) ...@@ -753,11 +754,14 @@ void tty_ldisc_hangup(struct tty_struct *tty)
* N_TTY. * N_TTY.
*/ */
if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) { if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) {
/* Make sure the old ldisc is quiescent */
tty_ldisc_halt(tty);
flush_scheduled_work();
/* Avoid racing set_ldisc or tty_ldisc_release */ /* Avoid racing set_ldisc or tty_ldisc_release */
mutex_lock(&tty->ldisc_mutex); mutex_lock(&tty->ldisc_mutex);
if (tty->ldisc) { /* Not yet closed */ if (tty->ldisc) { /* Not yet closed */
/* Switch back to N_TTY */ /* Switch back to N_TTY */
tty_ldisc_halt(tty);
tty_ldisc_reinit(tty); tty_ldisc_reinit(tty);
/* At this point we have a closed ldisc and we want to /* At this point we have a closed ldisc and we want to
reopen it. We could defer this to the next open but reopen it. We could defer this to the next open but
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment