diff options
author | Alan Cox <alan@linux.intel.com> | 2012-07-27 18:02:54 +0100 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2012-07-27 11:55:59 -0700 |
commit | d155255a344c417acad74156654295a2964e6b81 (patch) | |
tree | 71f7c39fda7d71980a64a5e1e35c08cf5c948820 /drivers/tty/tty_io.c | |
parent | 373f5aedbc6fb73d30f00eeb0dc7313ecfede908 (diff) | |
download | op-kernel-dev-d155255a344c417acad74156654295a2964e6b81.zip op-kernel-dev-d155255a344c417acad74156654295a2964e6b81.tar.gz |
tty: Fix race in tty release
Ian Abbott found that the tty layer would explode with the right set of
parallel open and close operations. This is because we race in the
handling of tty->drivers->termios[].
Correct this by
Making tty_ldisc_release behave like nromal code (takes the lock,
does stuff, drops the lock)
Drop the tty lock earlier in tty_ldisc_release
Taking the tty mutex around the driver->termios update in all cases
Adding a WARN_ON to catch future screwups.
I also forgot to clean up the pty resources properly. With a pty pair we
need to pull both halves out of the tables.
Signed-off-by: Alan Cox <alan@linux.intel.com>
Tested-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/tty/tty_io.c')
-rw-r--r-- | drivers/tty/tty_io.c | 20 |
1 files changed, 12 insertions, 8 deletions
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index be18d60..c6f4d71 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -1465,7 +1465,6 @@ EXPORT_SYMBOL(tty_free_termios); * in use. It also gets called when setup of a device fails. * * Locking: - * tty_mutex - sometimes only * takes the file list lock internally when working on the list * of ttys that the driver keeps. * @@ -1526,17 +1525,16 @@ EXPORT_SYMBOL(tty_kref_put); * and decrement the refcount of the backing module. * * Locking: - * tty_mutex - sometimes only + * tty_mutex * takes the file list lock internally when working on the list * of ttys that the driver keeps. - * FIXME: should we require tty_mutex is held here ?? * */ static void release_tty(struct tty_struct *tty, int idx) { /* This should always be true but check for the moment */ WARN_ON(tty->index != idx); - + WARN_ON(!mutex_is_locked(&tty_mutex)); if (tty->ops->shutdown) tty->ops->shutdown(tty); tty_free_termios(tty); @@ -1708,6 +1706,9 @@ int tty_release(struct inode *inode, struct file *filp) * The closing flags are now consistent with the open counts on * both sides, and we've completed the last operation that could * block, so it's safe to proceed with closing. + * + * We must *not* drop the tty_mutex until we ensure that a further + * entry into tty_open can not pick up this tty. */ if (pty_master) { if (--o_tty->count < 0) { @@ -1759,12 +1760,13 @@ int tty_release(struct inode *inode, struct file *filp) } mutex_unlock(&tty_mutex); + tty_unlock(); + /* At this point the TTY_CLOSING flag should ensure a dead tty + cannot be re-opened by a racing opener */ /* check whether both sides are closing ... */ - if (!tty_closing || (o_tty && !o_tty_closing)) { - tty_unlock(); + if (!tty_closing || (o_tty && !o_tty_closing)) return 0; - } #ifdef TTY_DEBUG_HANGUP printk(KERN_DEBUG "%s: freeing tty structure...\n", __func__); @@ -1777,12 +1779,14 @@ int tty_release(struct inode *inode, struct file *filp) * The release_tty function takes care of the details of clearing * the slots and preserving the termios structure. */ + mutex_lock(&tty_mutex); release_tty(tty, idx); + mutex_unlock(&tty_mutex); /* Make this pty number available for reallocation */ if (devpts) devpts_kill_index(inode, idx); - tty_unlock(); + return 0; } |