diff options
author | Jiri Slaby <jslaby@suse.cz> | 2010-11-29 04:16:54 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2010-11-29 17:52:48 -0500 |
commit | acfa747baf73922021a047f2d87a2d866f5dbab5 (patch) | |
tree | f47037d4c02341a9e4102eddd7dc6c774e6b439d /drivers/tty | |
parent | e2efafbf139d2bfdfe96f2901f03189fecd172e4 (diff) |
TTY: open/hangup race fixup
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>
Diffstat (limited to 'drivers/tty')
-rw-r--r-- | drivers/tty/tty_io.c | 10 |
1 files changed, 9 insertions, 1 deletions
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 878f6d667b17..35480dd57a30 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c | |||
@@ -559,6 +559,9 @@ void __tty_hangup(struct tty_struct *tty) | |||
559 | 559 | ||
560 | tty_lock(); | 560 | tty_lock(); |
561 | 561 | ||
562 | /* some functions below drop BTM, so we need this bit */ | ||
563 | set_bit(TTY_HUPPING, &tty->flags); | ||
564 | |||
562 | /* inuse_filps is protected by the single tty lock, | 565 | /* inuse_filps is protected by the single tty lock, |
563 | this really needs to change if we want to flush the | 566 | this really needs to change if we want to flush the |
564 | workqueue with the lock held */ | 567 | workqueue with the lock held */ |
@@ -578,6 +581,10 @@ void __tty_hangup(struct tty_struct *tty) | |||
578 | } | 581 | } |
579 | spin_unlock(&tty_files_lock); | 582 | spin_unlock(&tty_files_lock); |
580 | 583 | ||
584 | /* | ||
585 | * it drops BTM and thus races with reopen | ||
586 | * we protect the race by TTY_HUPPING | ||
587 | */ | ||
581 | tty_ldisc_hangup(tty); | 588 | tty_ldisc_hangup(tty); |
582 | 589 | ||
583 | read_lock(&tasklist_lock); | 590 | read_lock(&tasklist_lock); |
@@ -615,7 +622,6 @@ void __tty_hangup(struct tty_struct *tty) | |||
615 | tty->session = NULL; | 622 | tty->session = NULL; |
616 | tty->pgrp = NULL; | 623 | tty->pgrp = NULL; |
617 | tty->ctrl_status = 0; | 624 | tty->ctrl_status = 0; |
618 | set_bit(TTY_HUPPED, &tty->flags); | ||
619 | spin_unlock_irqrestore(&tty->ctrl_lock, flags); | 625 | spin_unlock_irqrestore(&tty->ctrl_lock, flags); |
620 | 626 | ||
621 | /* Account for the p->signal references we killed */ | 627 | /* Account for the p->signal references we killed */ |
@@ -641,6 +647,7 @@ void __tty_hangup(struct tty_struct *tty) | |||
641 | * can't yet guarantee all that. | 647 | * can't yet guarantee all that. |
642 | */ | 648 | */ |
643 | set_bit(TTY_HUPPED, &tty->flags); | 649 | set_bit(TTY_HUPPED, &tty->flags); |
650 | clear_bit(TTY_HUPPING, &tty->flags); | ||
644 | tty_ldisc_enable(tty); | 651 | tty_ldisc_enable(tty); |
645 | 652 | ||
646 | tty_unlock(); | 653 | tty_unlock(); |
@@ -1311,6 +1318,7 @@ static int tty_reopen(struct tty_struct *tty) | |||
1311 | struct tty_driver *driver = tty->driver; | 1318 | struct tty_driver *driver = tty->driver; |
1312 | 1319 | ||
1313 | if (test_bit(TTY_CLOSING, &tty->flags) || | 1320 | if (test_bit(TTY_CLOSING, &tty->flags) || |
1321 | test_bit(TTY_HUPPING, &tty->flags) || | ||
1314 | test_bit(TTY_LDISC_CHANGING, &tty->flags)) | 1322 | test_bit(TTY_LDISC_CHANGING, &tty->flags)) |
1315 | return -EIO; | 1323 | return -EIO; |
1316 | 1324 | ||