lkml.org 
[lkml]   [2010]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2 2/2] TTY: open/hangup race fixup
    Date
    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>
    ---
    I don't see a better way how to fix it, so I promoted the
    proof-of-concept to a real patch.

    drivers/tty/tty_io.c | 10 +++++++++-
    include/linux/tty.h | 1 +
    2 files changed, 10 insertions(+), 1 deletions(-)

    diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
    index 878f6d6..35480dd 100644
    --- a/drivers/tty/tty_io.c
    +++ b/drivers/tty/tty_io.c
    @@ -559,6 +559,9 @@ void __tty_hangup(struct tty_struct *tty)

    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,
    this really needs to change if we want to flush the
    workqueue with the lock held */
    @@ -578,6 +581,10 @@ void __tty_hangup(struct tty_struct *tty)
    }
    spin_unlock(&tty_files_lock);

    + /*
    + * it drops BTM and thus races with reopen
    + * we protect the race by TTY_HUPPING
    + */
    tty_ldisc_hangup(tty);

    read_lock(&tasklist_lock);
    @@ -615,7 +622,6 @@ void __tty_hangup(struct tty_struct *tty)
    tty->session = NULL;
    tty->pgrp = NULL;
    tty->ctrl_status = 0;
    - set_bit(TTY_HUPPED, &tty->flags);
    spin_unlock_irqrestore(&tty->ctrl_lock, flags);

    /* 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.
    */
    set_bit(TTY_HUPPED, &tty->flags);
    + clear_bit(TTY_HUPPING, &tty->flags);
    tty_ldisc_enable(tty);

    tty_unlock();
    @@ -1311,6 +1318,7 @@ static int tty_reopen(struct tty_struct *tty)
    struct tty_driver *driver = tty->driver;

    if (test_bit(TTY_CLOSING, &tty->flags) ||
    + test_bit(TTY_HUPPING, &tty->flags) ||
    test_bit(TTY_LDISC_CHANGING, &tty->flags))
    return -EIO;

    diff --git a/include/linux/tty.h b/include/linux/tty.h
    index 032d79f..54e4eaa 100644
    --- a/include/linux/tty.h
    +++ b/include/linux/tty.h
    @@ -366,6 +366,7 @@ struct tty_file_private {
    #define TTY_HUPPED 18 /* Post driver->hangup() */
    #define TTY_FLUSHING 19 /* Flushing to ldisc in progress */
    #define TTY_FLUSHPENDING 20 /* Queued buffer flush pending */
    +#define TTY_HUPPING 21 /* ->hangup() in progress */

    #define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))

    --
    1.7.3.1



    \
     
     \ /
      Last update: 2010-11-29 10:19    [W:0.030 / U:0.080 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site