From cf5284765862ac65e4a3e5b34652e593ffda2bdf Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Mon, 11 Mar 2013 16:44:28 -0400 Subject: tty: Strengthen no-subsequent-use guarantee of tty_ldisc_halt() In preparation for destructing and freeing the tty, the line discipline must first be brought to an inactive state before it can be destructed. This line discipline shutdown must: - disallow new users of the ldisc - wait for existing ldisc users to finish - only then, cancel/flush their pending/running work Factor tty_ldisc_wait_idle() from tty_set_ldisc() and tty_ldisc_kill() to ensure this shutdown order. Failure to provide this guarantee can result in scheduled work running after the tty has already been freed, as indicated in the following log message: [ 88.331234] WARNING: at drivers/tty/tty_buffer.c:435 flush_to_ldisc+0x194/0x1d0() [ 88.334505] Hardware name: Bochs [ 88.335618] tty is bad=-1 [ 88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth ...... [ 88.345272] Pid: 39, comm: kworker/1:1 Tainted: G W 3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug [ 88.347736] Call Trace: [ 88.349024] [] warn_slowpath_common+0x7f/0xc0 [ 88.350383] [] warn_slowpath_fmt+0x46/0x50 [ 88.351745] [] flush_to_ldisc+0x194/0x1d0 [ 88.353047] [] ? _raw_spin_unlock_irq+0x21/0x50 [ 88.354190] [] ? finish_task_switch+0x49/0xe0 [ 88.355436] [] process_one_work+0x121/0x490 [ 88.357674] [] ? __tty_buffer_flush+0x90/0x90 [ 88.358954] [] worker_thread+0x164/0x3e0 [ 88.360247] [] ? manage_workers+0x120/0x120 [ 88.361282] [] kthread+0xc0/0xd0 [ 88.362284] [] ? cmos_do_probe+0x2eb/0x3bf [ 88.363391] [] ? flush_kthread_worker+0xb0/0xb0 [ 88.364797] [] ret_from_fork+0x7c/0xb0 [ 88.366087] [] ? flush_kthread_worker+0xb0/0xb0 [ 88.367266] ---[ end trace 453a7c9f38fbfec0 ]--- Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_ldisc.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) (limited to 'drivers/tty/tty_ldisc.c') diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c index f691c7604d9a..525ee535c10d 100644 --- a/drivers/tty/tty_ldisc.c +++ b/drivers/tty/tty_ldisc.c @@ -530,24 +530,38 @@ static int tty_ldisc_wait_idle(struct tty_struct *tty, long timeout) /** * tty_ldisc_halt - shut down the line discipline * @tty: tty device + * @pending: returns true if work was scheduled when cancelled + * (can be set to NULL) + * @timeout: # of jiffies to wait for ldisc refs to be released * * Shut down the line discipline and work queue for this tty device. * The TTY_LDISC flag being cleared ensures no further references can * be obtained while the delayed work queue halt ensures that no more * data is fed to the ldisc. * + * Furthermore, guarantee that existing ldisc references have been + * released, which in turn, guarantees that no future buffer work + * can be rescheduled. + * * You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex) * 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, int *pending, long timeout) { - int scheduled; + int scheduled, retval; + clear_bit(TTY_LDISC, &tty->flags); + retval = tty_ldisc_wait_idle(tty, timeout); + if (retval) + return retval; + scheduled = cancel_work_sync(&tty->port->buf.work); set_bit(TTY_LDISC_HALTED, &tty->flags); - return scheduled; + if (pending) + *pending = scheduled; + return 0; } /** @@ -688,9 +702,9 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) * parallel to the change and re-referencing the tty. */ - work = tty_ldisc_halt(tty); - if (o_tty) - o_work = tty_ldisc_halt(o_tty); + retval = tty_ldisc_halt(tty, &work, 5 * HZ); + if (!retval && o_tty) + retval = tty_ldisc_halt(o_tty, &o_work, 5 * HZ); /* * Wait for ->hangup_work and ->buf.work handlers to terminate. @@ -701,8 +715,6 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) tty_ldisc_flush_works(tty); - retval = tty_ldisc_wait_idle(tty, 5 * HZ); - tty_lock(tty); mutex_lock(&tty->ldisc_mutex); @@ -921,11 +933,6 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty) static void tty_ldisc_kill(struct tty_struct *tty) { - /* There cannot be users from userspace now. But there still might be - * drivers holding a reference via tty_ldisc_ref. Do not steal them the - * ldisc until they are done. */ - tty_ldisc_wait_idle(tty, MAX_SCHEDULE_TIMEOUT); - mutex_lock(&tty->ldisc_mutex); /* * Now kill off the ldisc @@ -958,13 +965,12 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) * race with the set_ldisc code path. */ - tty_ldisc_halt(tty); - if (o_tty) - tty_ldisc_halt(o_tty); - + tty_ldisc_halt(tty, NULL, MAX_SCHEDULE_TIMEOUT); tty_ldisc_flush_works(tty); - if (o_tty) + if (o_tty) { + tty_ldisc_halt(o_tty, NULL, MAX_SCHEDULE_TIMEOUT); tty_ldisc_flush_works(o_tty); + } tty_lock_pair(tty, o_tty); /* This will need doing differently if we need to lock */ -- cgit v1.2.3