forge/linux-pinenote
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

TTY: open/hangup race fixup

Browse source 
Like in the "TTY: don't allow reopen when ldisc is changing" patch,
this one fixes a TTY WARNING as described in the option 1) there:
1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this
section tty_lock is held. However tty_lock is temporarily dropped in
the middle of the function by tty_ldisc_hangup.

The fix is to introduce a new flag which we set during the unlocked
window and check it in tty_reopen too. The flag is TTY_HUPPING and is
cleared after TTY_HUPPED is set.

While at it, remove duplicate TTY_HUPPED set_bit. The one after
calling ops->hangup seems to be more correct. But anyway, we hold
tty_lock, so there should be no difference.

Also document the function it does that kind of crap.

Nicely reproducible with two forked children:
static void do_work(const char *tty)
{
	if (signal(SIGHUP, SIG_IGN) == SIG_ERR) exit(1);
	setsid();
	while (1) {
		int fd = open(tty, O_RDWR|O_NOCTTY);
		if (fd < 0) continue;
		if (ioctl(fd, TIOCSCTTY)) continue;
		if (vhangup()) continue;
		close(fd);
	}
	exit(0);
}

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: <Valdis.Kletnieks@vt.edu>
Reported-by: Kyle McMartin <kyle@mcmartin.ca>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This commit is contained in:
Jiri Slaby 2010-11-29 10:16:54 +01:00 • committed by Greg Kroah-Hartman
commit acfa747baf
2 changed files with 10 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

10
drivers/tty/tty_io.c
View file

@ -559,6 +559,9 @@ void __tty_hangup(struct tty_struct *tty)
tty_lock(); tty_lock();
/* some functions below drop BTM, so we need this bit */
set_bit(TTY_HUPPING, &tty->flags);
/* inuse_filps is protected by the single tty lock, /* inuse_filps is protected by the single tty lock,
this really needs to change if we want to flush the this really needs to change if we want to flush the
workqueue with the lock held */ workqueue with the lock held */
@ -578,6 +581,10 @@ void __tty_hangup(struct tty_struct *tty)
} }
spin_unlock(&tty_files_lock); spin_unlock(&tty_files_lock);
/*
* it drops BTM and thus races with reopen
* we protect the race by TTY_HUPPING
*/
tty_ldisc_hangup(tty); tty_ldisc_hangup(tty);
read_lock(&tasklist_lock); read_lock(&tasklist_lock);
@ -615,7 +622,6 @@ void __tty_hangup(struct tty_struct *tty)
tty->session = NULL; tty->session = NULL;
tty->pgrp = NULL; tty->pgrp = NULL;
tty->ctrl_status = 0; tty->ctrl_status = 0;
set_bit(TTY_HUPPED, &tty->flags);
spin_unlock_irqrestore(&tty->ctrl_lock, flags); spin_unlock_irqrestore(&tty->ctrl_lock, flags);
/* Account for the p->signal references we killed */ /* Account for the p->signal references we killed */
@ -641,6 +647,7 @@ void __tty_hangup(struct tty_struct *tty)
* can't yet guarantee all that. * can't yet guarantee all that.
*/ */
set_bit(TTY_HUPPED, &tty->flags); set_bit(TTY_HUPPED, &tty->flags);
clear_bit(TTY_HUPPING, &tty->flags);
tty_ldisc_enable(tty); tty_ldisc_enable(tty);
tty_unlock(); tty_unlock();
@ -1311,6 +1318,7 @@ static int tty_reopen(struct tty_struct *tty)
struct tty_driver *driver = tty->driver; struct tty_driver *driver = tty->driver;
if (test_bit(TTY_CLOSING, &tty->flags) || if (test_bit(TTY_CLOSING, &tty->flags) ||
test_bit(TTY_HUPPING, &tty->flags) ||
test_bit(TTY_LDISC_CHANGING, &tty->flags)) test_bit(TTY_LDISC_CHANGING, &tty->flags))
return -EIO; return -EIO;

1
include/linux/tty.h
View file

@ -367,6 +367,7 @@ struct tty_file_private {
#define TTY_HUPPED 18 /* Post driver->hangup() */ #define TTY_HUPPED 18 /* Post driver->hangup() */
#define TTY_FLUSHING 19 /* Flushing to ldisc in progress */ #define TTY_FLUSHING 19 /* Flushing to ldisc in progress */
#define TTY_FLUSHPENDING 20 /* Queued buffer flush pending */ #define TTY_FLUSHPENDING 20 /* Queued buffer flush pending */
#define TTY_HUPPING 21 /* ->hangup() in progress */
#define TTY_WRITE_FLUSH(tty) tty_write_flush((tty)) #define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))