lkml.org 
[lkml]   [2010]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2010-12-13 10:29    [W:0.025 / U:0.184 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site