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>
This commit is contained in:
Alan Cox 2012-07-27 18:02:54 +01:00 коммит произвёл Greg Kroah-Hartman
Родитель 373f5aedbc
Коммит d155255a34
3 изменённых файлов: 37 добавлений и 11 удалений

Просмотреть файл

@ -282,6 +282,17 @@ done:
return 0;
}
/**
* pty_common_install - set up the pty pair
* @driver: the pty driver
* @tty: the tty being instantiated
* @bool: legacy, true if this is BSD style
*
* Perform the initial set up for the tty/pty pair. Called from the
* tty layer when the port is first opened.
*
* Locking: the caller must hold the tty_mutex
*/
static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty,
bool legacy)
{
@ -364,6 +375,14 @@ static int pty_install(struct tty_driver *driver, struct tty_struct *tty)
return pty_common_install(driver, tty, true);
}
static void pty_remove(struct tty_driver *driver, struct tty_struct *tty)
{
struct tty_struct *pair = tty->link;
driver->ttys[tty->index] = NULL;
if (pair)
pair->driver->ttys[pair->index] = NULL;
}
static int pty_bsd_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
@ -395,7 +414,8 @@ static const struct tty_operations master_pty_ops_bsd = {
.set_termios = pty_set_termios,
.ioctl = pty_bsd_ioctl,
.cleanup = pty_cleanup,
.resize = pty_resize
.resize = pty_resize,
.remove = pty_remove
};
static const struct tty_operations slave_pty_ops_bsd = {
@ -409,7 +429,8 @@ static const struct tty_operations slave_pty_ops_bsd = {
.unthrottle = pty_unthrottle,
.set_termios = pty_set_termios,
.cleanup = pty_cleanup,
.resize = pty_resize
.resize = pty_resize,
.remove = pty_remove
};
static void __init legacy_pty_init(void)

Просмотреть файл

@ -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;
}

Просмотреть файл

@ -912,7 +912,6 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)
* race with the set_ldisc code path.
*/
tty_unlock();
tty_ldisc_halt(tty);
tty_ldisc_flush_works(tty);
tty_lock();
@ -930,6 +929,8 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty)
tty_set_termios_ldisc(tty, N_TTY);
mutex_unlock(&tty->ldisc_mutex);
tty_unlock();
/* This will need doing differently if we need to lock */
if (o_tty)
tty_ldisc_release(o_tty, NULL);