[patch 02/26] tty: Avoid dropping ldisc_mutex over hangup tty re-initialization

From: Greg KH
Date: Fri Oct 09 2009 - 19:17:24 EST


2.6.31-stable review patch. If anyone has any objections, please let us know.

------------------
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>

commit 0b5759c654e74c8dc317ea2c6b3a7476160f688a upstream.

A couple of people have hit the WARN_ON() in drivers/char/tty_io.c,
tty_open() that is unhappy about seeing the tty line discipline go away
during the tty hangup. See for example

http://bugzilla.kernel.org/show_bug.cgi?id=14255

and the reason is that we do the tty_ldisc_halt() outside the
ldisc_mutex in order to be able to flush the scheduled work without a
deadlock with vhangup_work.

However, it turns out that we can solve this particular case by

- using "cancel_delayed_work_sync()" in tty_ldisc_halt(), which waits
for just the particular work, rather than synchronizing with any
random outstanding pending work.

This won't deadlock, since the buf.work we synchronize with doesn't
care about the ldisc_mutex, it just flushes the tty ldisc buffers.

- realize that for this particular case, we don't need to wait for any
hangup work, because we are inside the hangup codepaths ourselves.

so as a result we can just drop the flush_scheduled_work() entirely, and
then move the tty_ldisc_halt() call to inside the mutex. That way we
never expose the partially torn down ldisc state to tty_open(), and hold
the ldisc_mutex over the whole sequence.

Reported-by: Ingo Molnar <mingo@xxxxxxx>
Reported-by: Heinz Diehl <htd@xxxxxxxxxxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>

---
drivers/char/tty_ldisc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -516,7 +516,7 @@ static void tty_ldisc_restore(struct tty
static int tty_ldisc_halt(struct tty_struct *tty)
{
clear_bit(TTY_LDISC, &tty->flags);
- return cancel_delayed_work(&tty->buf.work);
+ return cancel_delayed_work_sync(&tty->buf.work);
}

/**
@@ -754,12 +754,9 @@ void tty_ldisc_hangup(struct tty_struct
* N_TTY.
*/
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 */
mutex_lock(&tty->ldisc_mutex);
+ tty_ldisc_halt(tty);
if (tty->ldisc) { /* Not yet closed */
/* Switch back to N_TTY */
tty_ldisc_reinit(tty);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/