| Date | Mon, 13 Dec 2010 10:26:19 +0100 | From | Jiri Slaby <> | Subject | Re: [PATCH] [98/223] TTY: open/hangup race fixup |
| |
On 12/13/2010 12:46 AM, Andi Kleen wrote: > 2.6.35-longterm review patch. If anyone has any objections, please let me know. > > ------------------ > From: Jiri Slaby <jslaby@suse.cz> > > commit acfa747baf73922021a047f2d87a2d866f5dbab5 upstream. > > 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> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > --- > drivers/char/tty_io.c | 10 +++++++++- > include/linux/tty.h | 1 + > 2 files changed, 10 insertions(+), 1 deletion(-) > > Index: linux/drivers/char/tty_io.c > =================================================================== > --- linux.orig/drivers/char/tty_io.c > +++ linux/drivers/char/tty_io.c > @@ -514,7 +514,10 @@ static void do_tty_hangup(struct work_st > spin_unlock(&redirect_lock); > > /* inuse_filps is protected by the single kernel lock */ > + > lock_kernel(); > + /* some functions below drop BTM, so we need this bit */ > + set_bit(TTY_HUPPING, &tty->flags);
This and 97/223 should not be needed in 2.6.35 -- there is BKL protecting the race. IOW BKL removal in 2.6.36 introduced the regression these 2 patches fix.
regards, -- js suse labs
|