From c65c9bc3efa5589f691276bb9db689119a711222 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Thu, 11 Jun 2009 12:50:12 +0100 Subject: [PATCH] tty: rewrite the ldisc locking There are several pretty much unfixable races in the old ldisc code, especially with respect to pty behaviour and also to hangup. It's easier to rewrite the code than simply try and patch it up. This patch - splits the ldisc from the tty (so we will be able to refcount it more cleanly later) - introduces a mutex lock for ldisc changing on an active device - fixes the complete mess that hangup caused - implements hopefully correct setldisc/close/hangup locking There are still some problems around pty pairs that have always been there but at least it is now possible to understand the code and fix further problems. This fixes the following known bugs - hang up can leak ldisc references - hang up may not call open/close on ldisc in a matched way - pty/tty pairs can deadlock during an ldisc change - reading the ldisc proc files can cause every ldisc to be loaded and probably a few other of the mysterious ldisc race reports. I'm sure it also adds the odd new one. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/bluetooth/hci_ldisc.c | 4 +- drivers/char/cyclades.c | 2 +- drivers/char/epca.c | 4 +- drivers/char/ip2/i2lib.c | 4 +- drivers/char/ip2/ip2main.c | 4 +- drivers/char/n_hdlc.c | 4 +- drivers/char/pty.c | 10 +- drivers/char/selection.c | 2 +- drivers/char/tty_io.c | 64 +---- drivers/char/tty_ldisc.c | 477 +++++++++++++++++++++------------- include/linux/tty.h | 9 +- 11 files changed, 330 insertions(+), 254 deletions(-) diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index af761dc434f6..688015128594 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -277,8 +277,8 @@ static int hci_uart_tty_open(struct tty_struct *tty) /* FIXME: why is this needed. Note don't use ldisc_ref here as the open path is before the ldisc is referencable */ - if (tty->ldisc.ops->flush_buffer) - tty->ldisc.ops->flush_buffer(tty); + if (tty->ldisc->ops->flush_buffer) + tty->ldisc->ops->flush_buffer(tty); tty_driver_flush_buffer(tty); return 0; diff --git a/drivers/char/cyclades.c b/drivers/char/cyclades.c index 456019051742..f3366d3f06cf 100644 --- a/drivers/char/cyclades.c +++ b/drivers/char/cyclades.c @@ -5200,7 +5200,7 @@ static int cyclades_proc_show(struct seq_file *m, void *v) (cur_jifs - info->idle_stats.recv_idle)/ HZ, info->idle_stats.overruns, /* FIXME: double check locking */ - (long)info->port.tty->ldisc.ops->num); + (long)info->port.tty->ldisc->ops->num); else seq_printf(m, "%3d %8lu %10lu %8lu " "%10lu %8lu %9lu %6ld\n", diff --git a/drivers/char/epca.c b/drivers/char/epca.c index 710ee9333057..abef1f7d84fe 100644 --- a/drivers/char/epca.c +++ b/drivers/char/epca.c @@ -2114,8 +2114,8 @@ static int pc_ioctl(struct tty_struct *tty, struct file *file, tty_wait_until_sent(tty, 0); } else { /* ldisc lock already held in ioctl */ - if (tty->ldisc.ops->flush_buffer) - tty->ldisc.ops->flush_buffer(tty); + if (tty->ldisc->ops->flush_buffer) + tty->ldisc->ops->flush_buffer(tty); } unlock_kernel(); /* Fall Thru */ diff --git a/drivers/char/ip2/i2lib.c b/drivers/char/ip2/i2lib.c index 0061e18aff60..0d10b89218ed 100644 --- a/drivers/char/ip2/i2lib.c +++ b/drivers/char/ip2/i2lib.c @@ -868,11 +868,11 @@ i2Input(i2ChanStrPtr pCh) amountToMove = count; } // Move the first block - pCh->pTTY->ldisc.ops->receive_buf( pCh->pTTY, + pCh->pTTY->ldisc->ops->receive_buf( pCh->pTTY, &(pCh->Ibuf[stripIndex]), NULL, amountToMove ); // If we needed to wrap, do the second data move if (count > amountToMove) { - pCh->pTTY->ldisc.ops->receive_buf( pCh->pTTY, + pCh->pTTY->ldisc->ops->receive_buf( pCh->pTTY, pCh->Ibuf, NULL, count - amountToMove ); } // Bump and wrap the stripIndex all at once by the amount of data read. This diff --git a/drivers/char/ip2/ip2main.c b/drivers/char/ip2/ip2main.c index afd9247cf082..517271c762e6 100644 --- a/drivers/char/ip2/ip2main.c +++ b/drivers/char/ip2/ip2main.c @@ -1315,8 +1315,8 @@ static inline void isig(int sig, struct tty_struct *tty, int flush) if (tty->pgrp) kill_pgrp(tty->pgrp, sig, 1); if (flush || !L_NOFLSH(tty)) { - if ( tty->ldisc.ops->flush_buffer ) - tty->ldisc.ops->flush_buffer(tty); + if ( tty->ldisc->ops->flush_buffer ) + tty->ldisc->ops->flush_buffer(tty); i2InputFlush( tty->driver_data ); } } diff --git a/drivers/char/n_hdlc.c b/drivers/char/n_hdlc.c index bacb3e2872ae..461ece591a5b 100644 --- a/drivers/char/n_hdlc.c +++ b/drivers/char/n_hdlc.c @@ -342,8 +342,8 @@ static int n_hdlc_tty_open (struct tty_struct *tty) #endif /* Flush any pending characters in the driver and discipline. */ - if (tty->ldisc.ops->flush_buffer) - tty->ldisc.ops->flush_buffer(tty); + if (tty->ldisc->ops->flush_buffer) + tty->ldisc->ops->flush_buffer(tty); tty_driver_flush_buffer(tty); diff --git a/drivers/char/pty.c b/drivers/char/pty.c index da2cb8c70c11..5acd29e6e043 100644 --- a/drivers/char/pty.c +++ b/drivers/char/pty.c @@ -110,7 +110,7 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, c = to->receive_room; if (c > count) c = count; - to->ldisc.ops->receive_buf(to, buf, NULL, c); + to->ldisc->ops->receive_buf(to, buf, NULL, c); return c; } @@ -148,11 +148,11 @@ static int pty_chars_in_buffer(struct tty_struct *tty) int count; /* We should get the line discipline lock for "tty->link" */ - if (!to || !to->ldisc.ops->chars_in_buffer) + if (!to || !to->ldisc->ops->chars_in_buffer) return 0; /* The ldisc must report 0 if no characters available to be read */ - count = to->ldisc.ops->chars_in_buffer(to); + count = to->ldisc->ops->chars_in_buffer(to); if (tty->driver->subtype == PTY_TYPE_SLAVE) return count; @@ -186,8 +186,8 @@ static void pty_flush_buffer(struct tty_struct *tty) if (!to) return; - if (to->ldisc.ops->flush_buffer) - to->ldisc.ops->flush_buffer(to); + if (to->ldisc->ops->flush_buffer) + to->ldisc->ops->flush_buffer(to); if (to->packet) { spin_lock_irqsave(&tty->ctrl_lock, flags); diff --git a/drivers/char/selection.c b/drivers/char/selection.c index cb8ca5698963..f97b9e848064 100644 --- a/drivers/char/selection.c +++ b/drivers/char/selection.c @@ -327,7 +327,7 @@ int paste_selection(struct tty_struct *tty) } count = sel_buffer_lth - pasted; count = min(count, tty->receive_room); - tty->ldisc.ops->receive_buf(tty, sel_buffer + pasted, + tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted, NULL, count); pasted += count; } diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c index be49d0730bb9..2f44b0b241c3 100644 --- a/drivers/char/tty_io.c +++ b/drivers/char/tty_io.c @@ -491,22 +491,6 @@ void tty_ldisc_flush(struct tty_struct *tty) EXPORT_SYMBOL_GPL(tty_ldisc_flush); -/** - * tty_reset_termios - reset terminal state - * @tty: tty to reset - * - * Restore a terminal to the driver default state - */ - -static void tty_reset_termios(struct tty_struct *tty) -{ - mutex_lock(&tty->termios_mutex); - *tty->termios = tty->driver->init_termios; - tty->termios->c_ispeed = tty_termios_input_baud_rate(tty->termios); - tty->termios->c_ospeed = tty_termios_baud_rate(tty->termios); - mutex_unlock(&tty->termios_mutex); -} - /** * do_tty_hangup - actual handler for hangup events * @work: tty device @@ -536,7 +520,6 @@ static void do_tty_hangup(struct work_struct *work) struct file *cons_filp = NULL; struct file *filp, *f = NULL; struct task_struct *p; - struct tty_ldisc *ld; int closecount = 0, n; unsigned long flags; int refs = 0; @@ -567,40 +550,8 @@ static void do_tty_hangup(struct work_struct *work) filp->f_op = &hung_up_tty_fops; } file_list_unlock(); - /* - * FIXME! What are the locking issues here? This may me overdoing - * things... This question is especially important now that we've - * removed the irqlock. - */ - ld = tty_ldisc_ref(tty); - if (ld != NULL) { - /* We may have no line discipline at this point */ - if (ld->ops->flush_buffer) - ld->ops->flush_buffer(tty); - tty_driver_flush_buffer(tty); - if ((test_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) && - ld->ops->write_wakeup) - ld->ops->write_wakeup(tty); - if (ld->ops->hangup) - ld->ops->hangup(tty); - } - /* - * FIXME: Once we trust the LDISC code better we can wait here for - * ldisc completion and fix the driver call race - */ - wake_up_interruptible_poll(&tty->write_wait, POLLOUT); - wake_up_interruptible_poll(&tty->read_wait, POLLIN); - /* - * Shutdown the current line discipline, and reset it to - * N_TTY. - */ - if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) - tty_reset_termios(tty); - /* Defer ldisc switch */ - /* tty_deferred_ldisc_switch(N_TTY); - This should get done automatically when the port closes and - tty_release is called */ + tty_ldisc_hangup(tty); read_lock(&tasklist_lock); if (tty->session) { @@ -629,12 +580,15 @@ static void do_tty_hangup(struct work_struct *work) read_unlock(&tasklist_lock); spin_lock_irqsave(&tty->ctrl_lock, flags); - tty->flags = 0; + clear_bit(TTY_THROTTLED, &tty->flags); + clear_bit(TTY_PUSH, &tty->flags); + clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags); put_pid(tty->session); put_pid(tty->pgrp); tty->session = NULL; tty->pgrp = NULL; tty->ctrl_status = 0; + set_bit(TTY_HUPPED, &tty->flags); spin_unlock_irqrestore(&tty->ctrl_lock, flags); /* Account for the p->signal references we killed */ @@ -660,10 +614,7 @@ static void do_tty_hangup(struct work_struct *work) * can't yet guarantee all that. */ set_bit(TTY_HUPPED, &tty->flags); - if (ld) { - tty_ldisc_enable(tty); - tty_ldisc_deref(ld); - } + tty_ldisc_enable(tty); unlock_kernel(); if (f) fput(f); @@ -2570,7 +2521,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case TIOCGSID: return tiocgsid(tty, real_tty, p); case TIOCGETD: - return put_user(tty->ldisc.ops->num, (int __user *)p); + return put_user(tty->ldisc->ops->num, (int __user *)p); case TIOCSETD: return tiocsetd(tty, p); /* @@ -2785,6 +2736,7 @@ void initialize_tty_struct(struct tty_struct *tty, tty->buf.head = tty->buf.tail = NULL; tty_buffer_init(tty); mutex_init(&tty->termios_mutex); + mutex_init(&tty->ldisc_mutex); init_waitqueue_head(&tty->write_wait); init_waitqueue_head(&tty->read_wait); INIT_WORK(&tty->hangup_work, do_tty_hangup); diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index e3c6416aa86d..a58a19a6a5e1 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -115,19 +115,22 @@ EXPORT_SYMBOL(tty_unregister_ldisc); /** * tty_ldisc_try_get - try and reference an ldisc * @disc: ldisc number - * @ld: tty ldisc structure to complete * * Attempt to open and lock a line discipline into place. Return - * the line discipline refcounted and assigned in ld. On an error - * report the error code back + * the line discipline refcounted or an error. */ -static int tty_ldisc_try_get(int disc, struct tty_ldisc *ld) +static struct tty_ldisc *tty_ldisc_try_get(int disc) { unsigned long flags; + struct tty_ldisc *ld; struct tty_ldisc_ops *ldops; int err = -EINVAL; + ld = kmalloc(sizeof(struct tty_ldisc), GFP_KERNEL); + if (ld == NULL) + return ERR_PTR(-ENOMEM); + spin_lock_irqsave(&tty_ldisc_lock, flags); ld->ops = NULL; ldops = tty_ldiscs[disc]; @@ -140,17 +143,19 @@ static int tty_ldisc_try_get(int disc, struct tty_ldisc *ld) /* lock it */ ldops->refcount++; ld->ops = ldops; + ld->refcount = 0; err = 0; } } spin_unlock_irqrestore(&tty_ldisc_lock, flags); - return err; + if (err) + return ERR_PTR(err); + return ld; } /** * tty_ldisc_get - take a reference to an ldisc * @disc: ldisc number - * @ld: tty line discipline structure to use * * Takes a reference to a line discipline. Deals with refcounts and * module locking counts. Returns NULL if the discipline is not available. @@ -161,44 +166,46 @@ static int tty_ldisc_try_get(int disc, struct tty_ldisc *ld) * takes tty_ldisc_lock to guard against ldisc races */ -static int tty_ldisc_get(int disc, struct tty_ldisc *ld) +static struct tty_ldisc *tty_ldisc_get(int disc) { - int err; + struct tty_ldisc *ld; if (disc < N_TTY || disc >= NR_LDISCS) - return -EINVAL; - err = tty_ldisc_try_get(disc, ld); - if (err < 0) { + return ERR_PTR(-EINVAL); + ld = tty_ldisc_try_get(disc); + if (IS_ERR(ld)) { request_module("tty-ldisc-%d", disc); - err = tty_ldisc_try_get(disc, ld); + ld = tty_ldisc_try_get(disc); } - return err; + return ld; } /** * tty_ldisc_put - drop ldisc reference - * @disc: ldisc number + * @ld: ldisc * * Drop a reference to a line discipline. Manage refcounts and - * module usage counts + * module usage counts. Free the ldisc once the recount hits zero. * * Locking: * takes tty_ldisc_lock to guard against ldisc races */ -static void tty_ldisc_put(struct tty_ldisc_ops *ld) +static void tty_ldisc_put(struct tty_ldisc *ld) { unsigned long flags; - int disc = ld->num; + int disc = ld->ops->num; + struct tty_ldisc_ops *ldo; BUG_ON(disc < N_TTY || disc >= NR_LDISCS); spin_lock_irqsave(&tty_ldisc_lock, flags); - ld = tty_ldiscs[disc]; - BUG_ON(ld->refcount == 0); - ld->refcount--; - module_put(ld->owner); + ldo = tty_ldiscs[disc]; + BUG_ON(ldo->refcount == 0); + ldo->refcount--; + module_put(ldo->owner); spin_unlock_irqrestore(&tty_ldisc_lock, flags); + kfree(ld); } static void * tty_ldiscs_seq_start(struct seq_file *m, loff_t *pos) @@ -219,12 +226,13 @@ static void tty_ldiscs_seq_stop(struct seq_file *m, void *v) static int tty_ldiscs_seq_show(struct seq_file *m, void *v) { int i = *(loff_t *)v; - struct tty_ldisc ld; + struct tty_ldisc *ld; - if (tty_ldisc_get(i, &ld) < 0) + ld = tty_ldisc_try_get(i); + if (IS_ERR(ld)) return 0; - seq_printf(m, "%-10s %2d\n", ld.ops->name ? ld.ops->name : "???", i); - tty_ldisc_put(ld.ops); + seq_printf(m, "%-10s %2d\n", ld->ops->name ? ld->ops->name : "???", i); + tty_ldisc_put(ld); return 0; } @@ -263,8 +271,7 @@ const struct file_operations tty_ldiscs_proc_fops = { static void tty_ldisc_assign(struct tty_struct *tty, struct tty_ldisc *ld) { - ld->refcount = 0; - tty->ldisc = *ld; + tty->ldisc = ld; } /** @@ -286,7 +293,7 @@ static int tty_ldisc_try(struct tty_struct *tty) int ret = 0; spin_lock_irqsave(&tty_ldisc_lock, flags); - ld = &tty->ldisc; + ld = tty->ldisc; if (test_bit(TTY_LDISC, &tty->flags)) { ld->refcount++; ret = 1; @@ -315,8 +322,8 @@ struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty) { /* wait_event is a macro */ wait_event(tty_ldisc_wait, tty_ldisc_try(tty)); - WARN_ON(tty->ldisc.refcount == 0); - return &tty->ldisc; + WARN_ON(tty->ldisc->refcount == 0); + return tty->ldisc; } EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait); @@ -335,7 +342,7 @@ EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait); struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty) { if (tty_ldisc_try(tty)) - return &tty->ldisc; + return tty->ldisc; return NULL; } @@ -407,6 +414,39 @@ static void tty_set_termios_ldisc(struct tty_struct *tty, int num) mutex_unlock(&tty->termios_mutex); } +/** + * tty_ldisc_open - open a line discipline + * @tty: tty we are opening the ldisc on + * @ld: discipline to open + * + * A helper opening method. Also a convenient debugging and check + * point. + */ + +static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld) +{ + WARN_ON(test_and_set_bit(TTY_LDISC_OPEN, &tty->flags)); + if (ld->ops->open) + return ld->ops->open(tty); + return 0; +} + +/** + * tty_ldisc_close - close a line discipline + * @tty: tty we are opening the ldisc on + * @ld: discipline to close + * + * A helper close method. Also a convenient debugging and check + * point. + */ + +static void tty_ldisc_close(struct tty_struct *tty, struct tty_ldisc *ld) +{ + WARN_ON(!test_bit(TTY_LDISC_OPEN, &tty->flags)); + clear_bit(TTY_LDISC_OPEN, &tty->flags); + if (ld->ops->close) + ld->ops->close(tty); +} /** * tty_ldisc_restore - helper for tty ldisc change @@ -420,31 +460,32 @@ static void tty_set_termios_ldisc(struct tty_struct *tty, int num) static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old) { char buf[64]; - struct tty_ldisc new_ldisc; + struct tty_ldisc *new_ldisc; + int r; /* There is an outstanding reference here so this is safe */ - tty_ldisc_get(old->ops->num, old); + old = tty_ldisc_get(old->ops->num); + WARN_ON(IS_ERR(old)); tty_ldisc_assign(tty, old); tty_set_termios_ldisc(tty, old->ops->num); - if (old->ops->open && (old->ops->open(tty) < 0)) { - tty_ldisc_put(old->ops); + if (tty_ldisc_open(tty, old) < 0) { + tty_ldisc_put(old); /* This driver is always present */ - if (tty_ldisc_get(N_TTY, &new_ldisc) < 0) + new_ldisc =tty_ldisc_get(N_TTY); + if (IS_ERR(new_ldisc)) panic("n_tty: get"); - tty_ldisc_assign(tty, &new_ldisc); + tty_ldisc_assign(tty, new_ldisc); tty_set_termios_ldisc(tty, N_TTY); - if (new_ldisc.ops->open) { - int r = new_ldisc.ops->open(tty); - if (r < 0) - panic("Couldn't open N_TTY ldisc for " - "%s --- error %d.", - tty_name(tty, buf), r); - } + r = tty_ldisc_open(tty, new_ldisc); + if (r < 0) + panic("Couldn't open N_TTY ldisc for " + "%s --- error %d.", + tty_name(tty, buf), r); } } /** - * tty_ldisc_halt - shutdown the line discipline + * tty_ldisc_halt - shut down the line discipline * @tty: tty device * * Shut down the line discipline and work queue for this tty device. @@ -456,14 +497,10 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old) * tty_ldisc_wait_idle. */ -static void tty_ldisc_halt(struct tty_struct *tty) +static int tty_ldisc_halt(struct tty_struct *tty) { clear_bit(TTY_LDISC, &tty->flags); - cancel_delayed_work(&tty->buf.work); - /* - * Wait for ->hangup_work and ->buf.work handlers to terminate - */ - flush_scheduled_work(); + return cancel_delayed_work(&tty->buf.work); } /** @@ -473,18 +510,22 @@ static void tty_ldisc_halt(struct tty_struct *tty) * Wait for the line discipline to become idle. The discipline must * have been halted for this to guarantee it remains idle. * + * tty_ldisc_lock protects the ref counts currently. */ -static void tty_ldisc_wait_idle(struct tty_struct *tty) +static int tty_ldisc_wait_idle(struct tty_struct *tty) { unsigned long flags; spin_lock_irqsave(&tty_ldisc_lock, flags); - while (tty->ldisc.refcount) { + while (tty->ldisc->refcount) { spin_unlock_irqrestore(&tty_ldisc_lock, flags); - wait_event(tty_ldisc_wait, tty->ldisc.refcount == 0); + if (wait_event_timeout(tty_ldisc_wait, + tty->ldisc->refcount == 0, 5 * HZ) == 0) + return -EBUSY; spin_lock_irqsave(&tty_ldisc_lock, flags); } spin_unlock_irqrestore(&tty_ldisc_lock, flags); + return 0; } /** @@ -493,38 +534,63 @@ static void tty_ldisc_wait_idle(struct tty_struct *tty) * @ldisc: the line discipline * * Set the discipline of a tty line. Must be called from a process - * context. + * context. The ldisc change logic has to protect itself against any + * overlapping ldisc change (including on the other end of pty pairs), + * the close of one side of a tty/pty pair, and eventually hangup. * - * Locking: takes tty_ldisc_lock. - * called functions take termios_mutex + * Locking: takes tty_ldisc_lock, termios_mutex */ int tty_set_ldisc(struct tty_struct *tty, int ldisc) { int retval; - struct tty_ldisc o_ldisc, new_ldisc; - int work; - unsigned long flags; + struct tty_ldisc *o_ldisc, *new_ldisc; + int work, o_work = 0; struct tty_struct *o_tty; -restart: - /* This is a bit ugly for now but means we can break the 'ldisc - is part of the tty struct' assumption later */ - retval = tty_ldisc_get(ldisc, &new_ldisc); - if (retval) - return retval; + new_ldisc = tty_ldisc_get(ldisc); + if (IS_ERR(new_ldisc)) + return PTR_ERR(new_ldisc); /* - * Problem: What do we do if this blocks ? + * We need to look at the tty locking here for pty/tty pairs + * when both sides try to change in parallel. */ - tty_wait_until_sent(tty, 0); + o_tty = tty->link; /* o_tty is the pty side or NULL */ + - if (tty->ldisc.ops->num == ldisc) { - tty_ldisc_put(new_ldisc.ops); + /* + * Check the no-op case + */ + + if (tty->ldisc->ops->num == ldisc) { + tty_ldisc_put(new_ldisc); return 0; } + /* + * Problem: What do we do if this blocks ? + * We could deadlock here + */ + + tty_wait_until_sent(tty, 0); + + mutex_lock(&tty->ldisc_mutex); + + /* + * We could be midstream of another ldisc change which has + * dropped the lock during processing. If so we need to wait. + */ + + while (test_bit(TTY_LDISC_CHANGING, &tty->flags)) { + mutex_unlock(&tty->ldisc_mutex); + wait_event(tty_ldisc_wait, + test_bit(TTY_LDISC_CHANGING, &tty->flags) == 0); + mutex_lock(&tty->ldisc_mutex); + } + set_bit(TTY_LDISC_CHANGING, &tty->flags); + /* * No more input please, we are switching. The new ldisc * will update this value in the ldisc open function @@ -533,8 +599,6 @@ restart: tty->receive_room = 0; o_ldisc = tty->ldisc; - o_tty = tty->link; - /* * Make sure we don't change while someone holds a * reference to the line discipline. The TTY_LDISC bit @@ -545,108 +609,181 @@ restart: * with a userspace app continually trying to use the tty in * parallel to the change and re-referencing the tty. */ - clear_bit(TTY_LDISC, &tty->flags); - if (o_tty) - clear_bit(TTY_LDISC, &o_tty->flags); - spin_lock_irqsave(&tty_ldisc_lock, flags); - if (tty->ldisc.refcount || (o_tty && o_tty->ldisc.refcount)) { - if (tty->ldisc.refcount) { - /* Free the new ldisc we grabbed. Must drop the lock - first. */ - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - tty_ldisc_put(o_ldisc.ops); - /* - * There are several reasons we may be busy, including - * random momentary I/O traffic. We must therefore - * retry. We could distinguish between blocking ops - * and retries if we made tty_ldisc_wait() smarter. - * That is up for discussion. - */ - if (wait_event_interruptible(tty_ldisc_wait, tty->ldisc.refcount == 0) < 0) - return -ERESTARTSYS; - goto restart; - } - if (o_tty && o_tty->ldisc.refcount) { - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - tty_ldisc_put(o_tty->ldisc.ops); - if (wait_event_interruptible(tty_ldisc_wait, o_tty->ldisc.refcount == 0) < 0) - return -ERESTARTSYS; - goto restart; - } - } - /* - * If the TTY_LDISC bit is set, then we are racing against - * another ldisc change - */ - if (test_bit(TTY_LDISC_CHANGING, &tty->flags)) { - struct tty_ldisc *ld; - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - tty_ldisc_put(new_ldisc.ops); - ld = tty_ldisc_ref_wait(tty); - tty_ldisc_deref(ld); - goto restart; - } - /* - * This flag is used to avoid two parallel ldisc changes. Once - * open and close are fine grained locked this may work better - * as a mutex shared with the open/close/hup paths - */ - set_bit(TTY_LDISC_CHANGING, &tty->flags); + work = tty_ldisc_halt(tty); if (o_tty) - set_bit(TTY_LDISC_CHANGING, &o_tty->flags); - spin_unlock_irqrestore(&tty_ldisc_lock, flags); - - /* - * From this point on we know nobody has an ldisc - * usage reference, nor can they obtain one until - * we say so later on. - */ + o_work = tty_ldisc_halt(o_tty); - work = cancel_delayed_work(&tty->buf.work); /* - * Wait for ->hangup_work and ->buf.work handlers to terminate - * MUST NOT hold locks here. + * Wait for ->hangup_work and ->buf.work handlers to terminate. + * We must drop the mutex here in case a hangup is also in process. */ + + mutex_unlock(&tty->ldisc_mutex); + flush_scheduled_work(); + + /* Let any existing reference holders finish */ + retval = tty_ldisc_wait_idle(tty); + if (retval < 0) { + clear_bit(TTY_LDISC_CHANGING, &tty->flags); + tty_ldisc_put(new_ldisc); + return retval; + } + + mutex_lock(&tty->ldisc_mutex); + if (test_bit(TTY_HUPPED, &tty->flags)) { + /* We were raced by the hangup method. It will have stomped + the ldisc data and closed the ldisc down */ + clear_bit(TTY_LDISC_CHANGING, &tty->flags); + mutex_unlock(&tty->ldisc_mutex); + tty_ldisc_put(new_ldisc); + return -EIO; + } + /* Shutdown the current discipline. */ - if (o_ldisc.ops->close) - (o_ldisc.ops->close)(tty); + tty_ldisc_close(tty, o_ldisc); /* Now set up the new line discipline. */ - tty_ldisc_assign(tty, &new_ldisc); + tty_ldisc_assign(tty, new_ldisc); tty_set_termios_ldisc(tty, ldisc); - if (new_ldisc.ops->open) - retval = (new_ldisc.ops->open)(tty); + + retval = tty_ldisc_open(tty, new_ldisc); if (retval < 0) { - tty_ldisc_put(new_ldisc.ops); - tty_ldisc_restore(tty, &o_ldisc); + /* Back to the old one or N_TTY if we can't */ + tty_ldisc_put(new_ldisc); + tty_ldisc_restore(tty, o_ldisc); } + /* At this point we hold a reference to the new ldisc and a a reference to the old ldisc. If we ended up flipping back to the existing ldisc we have two references to it */ - if (tty->ldisc.ops->num != o_ldisc.ops->num && tty->ops->set_ldisc) + if (tty->ldisc->ops->num != o_ldisc->ops->num && tty->ops->set_ldisc) tty->ops->set_ldisc(tty); - tty_ldisc_put(o_ldisc.ops); + tty_ldisc_put(o_ldisc); /* - * Allow ldisc referencing to occur as soon as the driver - * ldisc callback completes. + * Allow ldisc referencing to occur again */ tty_ldisc_enable(tty); if (o_tty) tty_ldisc_enable(o_tty); - /* Restart it in case no characters kick it off. Safe if + /* Restart the work queue in case no characters kick it off. Safe if already running */ if (work) schedule_delayed_work(&tty->buf.work, 1); + if (o_work) + schedule_delayed_work(&o_tty->buf.work, 1); + mutex_unlock(&tty->ldisc_mutex); return retval; } +/** + * tty_reset_termios - reset terminal state + * @tty: tty to reset + * + * Restore a terminal to the driver default state. + */ + +static void tty_reset_termios(struct tty_struct *tty) +{ + mutex_lock(&tty->termios_mutex); + *tty->termios = tty->driver->init_termios; + tty->termios->c_ispeed = tty_termios_input_baud_rate(tty->termios); + tty->termios->c_ospeed = tty_termios_baud_rate(tty->termios); + mutex_unlock(&tty->termios_mutex); +} + + +/** + * tty_ldisc_reinit - reinitialise the tty ldisc + * @tty: tty to reinit + * + * Switch the tty back to N_TTY line discipline and leave the + * ldisc state closed + */ + +static void tty_ldisc_reinit(struct tty_struct *tty) +{ + struct tty_ldisc *ld; + + tty_ldisc_close(tty, tty->ldisc); + tty_ldisc_put(tty->ldisc); + tty->ldisc = NULL; + /* + * Switch the line discipline back + */ + ld = tty_ldisc_get(N_TTY); + BUG_ON(IS_ERR(ld)); + tty_ldisc_assign(tty, ld); + tty_set_termios_ldisc(tty, N_TTY); +} + +/** + * tty_ldisc_hangup - hangup ldisc reset + * @tty: tty being hung up + * + * Some tty devices reset their termios when they receive a hangup + * event. In that situation we must also switch back to N_TTY properly + * before we reset the termios data. + * + * Locking: We can take the ldisc mutex as the rest of the code is + * careful to allow for this. + * + * In the pty pair case this occurs in the close() path of the + * tty itself so we must be careful about locking rules. + */ + +void tty_ldisc_hangup(struct tty_struct *tty) +{ + struct tty_ldisc *ld; + + /* + * FIXME! What are the locking issues here? This may me overdoing + * things... This question is especially important now that we've + * removed the irqlock. + */ + ld = tty_ldisc_ref(tty); + if (ld != NULL) { + /* We may have no line discipline at this point */ + if (ld->ops->flush_buffer) + ld->ops->flush_buffer(tty); + tty_driver_flush_buffer(tty); + if ((test_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) && + ld->ops->write_wakeup) + ld->ops->write_wakeup(tty); + if (ld->ops->hangup) + ld->ops->hangup(tty); + tty_ldisc_deref(ld); + } + /* + * FIXME: Once we trust the LDISC code better we can wait here for + * ldisc completion and fix the driver call race + */ + wake_up_interruptible_poll(&tty->write_wait, POLLOUT); + wake_up_interruptible_poll(&tty->read_wait, POLLIN); + /* + * Shutdown the current line discipline, and reset it to + * N_TTY. + */ + if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) { + /* Avoid racing set_ldisc */ + mutex_lock(&tty->ldisc_mutex); + /* Switch back to N_TTY */ + tty_ldisc_reinit(tty); + /* At this point we have a closed ldisc and we want to + reopen it. We could defer this to the next open but + it means auditing a lot of other paths so this is a FIXME */ + WARN_ON(tty_ldisc_open(tty, tty->ldisc)); + tty_ldisc_enable(tty); + mutex_unlock(&tty->ldisc_mutex); + tty_reset_termios(tty); + } +} /** * tty_ldisc_setup - open line discipline @@ -654,24 +791,23 @@ restart: * @o_tty: pair tty for pty/tty pairs * * Called during the initial open of a tty/pty pair in order to set up the - * line discplines and bind them to the tty. + * line disciplines and bind them to the tty. This has no locking issues + * as the device isn't yet active. */ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty) { - struct tty_ldisc *ld = &tty->ldisc; + struct tty_ldisc *ld = tty->ldisc; int retval; - if (ld->ops->open) { - retval = (ld->ops->open)(tty); - if (retval) - return retval; - } - if (o_tty && o_tty->ldisc.ops->open) { - retval = (o_tty->ldisc.ops->open)(o_tty); + retval = tty_ldisc_open(tty, ld); + if (retval) + return retval; + + if (o_tty) { + retval = tty_ldisc_open(o_tty, o_tty->ldisc); if (retval) { - if (ld->ops->close) - (ld->ops->close)(tty); + tty_ldisc_close(tty, ld); return retval; } tty_ldisc_enable(o_tty); @@ -679,34 +815,18 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty) tty_ldisc_enable(tty); return 0; } - -static void tty_ldisc_reinit(struct tty_struct *tty) -{ - struct tty_ldisc ld; - - if (tty->ldisc.ops->close) - (tty->ldisc.ops->close)(tty); - tty_ldisc_put(tty->ldisc.ops); - /* - * Switch the line discipline back - */ - WARN_ON(tty_ldisc_get(N_TTY, &ld)); - tty_ldisc_assign(tty, &ld); - tty_set_termios_ldisc(tty, N_TTY); -} - /** * tty_ldisc_release - release line discipline * @tty: tty being shut down * @o_tty: pair tty for pty/tty pairs * * Called during the final close of a tty/pty pair in order to shut down the - * line discpline layer. + * line discpline layer. On exit the ldisc assigned is N_TTY and the + * ldisc has not been opened. */ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) { - /* * Prevent flush_to_ldisc() from rescheduling the work for later. Then * kill any delayed work. As this is the final close it does not @@ -714,6 +834,7 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) */ tty_ldisc_halt(tty); + flush_scheduled_work(); /* * Wait for any short term users (we know they are just driver @@ -730,11 +851,9 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) */ tty_ldisc_reinit(tty); - if (o_tty) { - /* FIXME: could o_tty be in setldisc here ? */ - clear_bit(TTY_LDISC, &o_tty->flags); - tty_ldisc_reinit(o_tty); - } + /* This will need doing differently if we need to lock */ + if (o_tty) + tty_ldisc_release(o_tty, NULL); } /** @@ -747,10 +866,10 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) void tty_ldisc_init(struct tty_struct *tty) { - struct tty_ldisc ld; - if (tty_ldisc_get(N_TTY, &ld) < 0) + struct tty_ldisc *ld = tty_ldisc_get(N_TTY); + if (IS_ERR(ld)) panic("n_tty: init_tty"); - tty_ldisc_assign(tty, &ld); + tty_ldisc_assign(tty, ld); } void tty_ldisc_begin(void) diff --git a/include/linux/tty.h b/include/linux/tty.h index f9c13c83790c..1488d8c81aac 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -226,8 +226,11 @@ struct tty_struct { struct tty_driver *driver; const struct tty_operations *ops; int index; - /* The ldisc objects are protected by tty_ldisc_lock at the moment */ - struct tty_ldisc ldisc; + + /* Protects ldisc changes: Lock tty not pty */ + struct mutex ldisc_mutex; + struct tty_ldisc *ldisc; + struct mutex termios_mutex; spinlock_t ctrl_lock; /* Termios values are protected by the termios mutex */ @@ -314,6 +317,7 @@ struct tty_struct { #define TTY_CLOSING 7 /* ->close() in progress */ #define TTY_LDISC 9 /* Line discipline attached */ #define TTY_LDISC_CHANGING 10 /* Line discipline changing */ +#define TTY_LDISC_OPEN 11 /* Line discipline is open */ #define TTY_HW_COOK_OUT 14 /* Hardware can do output cooking */ #define TTY_HW_COOK_IN 15 /* Hardware can do input cooking */ #define TTY_PTY_LOCK 16 /* pty private */ @@ -406,6 +410,7 @@ extern int tty_termios_hw_change(struct ktermios *a, struct ktermios *b); extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *); extern void tty_ldisc_deref(struct tty_ldisc *); extern struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *); +extern void tty_ldisc_hangup(struct tty_struct *tty); extern const struct file_operations tty_ldiscs_proc_fops; extern void tty_wakeup(struct tty_struct *tty); -- 2.39.5