lkml.org 
[lkml]   [2013]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] tty: fix ldisc flush and termios setting race
    From: Min Zhang <mzhang@mvista.com>

    A race condition can clear tty ldisc icanon bit unintentionally which
    could stop n_tty from processing received characters. It can occur when
    tty receiver buffer was full, e.g. 4096 chars received, 8250 serial driver
    interrupt tried to flush_to_ldisc them, but other shell thread tried
    to change_termios the tty setting too. Then n_tty_receive_char and
    n_tty_set_termios both tried to modify n_tty_data fields.

    Specifically the icanon and its neighbor fields are defines as bits:

    unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1;

    However they are not atomically accessed in the follow race condition:

    serial8250_handle_irq
    serail8250_rx_chars
    tty_flip_buffer_push
    schdule_work -------> flush_to_ldisc
    n_tty_receive_buf
    n_tty_receive_char
    (holds no lock)
    ioctl
    set_termios
    tty_set_termios
    n_tty_set_termios
    (holds termios_mutex)

    The patch let change_termios to use TTY_LDISC_FLUSHING to prevent
    flush_to_ldisc from running, and flush_to_ldisc use TTY_FLUSHING
    to make change_termios wait until the flag is cleared.

    The patch also replaces flush_to_ldisc's wake_up with wake_up_all,
    because it can now wake up both TTY_FLUSHING and TTY_FLUSHPENDING
    on the same tty->read_wait queue. Event waiters need check which event.

    Signed-off-by: Min Zhang <mzhang@mvista.com>
    ---
    drivers/tty/tty_buffer.c | 12 +++++++++++-
    drivers/tty/tty_ioctl.c | 27 ++++++++++++++++++++++++++-
    2 files changed, 37 insertions(+), 2 deletions(-)

    diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
    index 45d9161..37f4818 100644
    --- a/drivers/tty/tty_buffer.c
    +++ b/drivers/tty/tty_buffer.c
    @@ -482,6 +482,13 @@ static void flush_to_ldisc(struct work_struct *work)

    spin_lock_irqsave(&buf->lock, flags);

    + /* Ldisc change by change_termios can race with ldisc receive_buf, esp
    + to access N_TTY line discipline field in tty_struct, so we defer */
    + if (test_bit(TTY_LDISC_CHANGING, &tty->flags)) {
    + schedule_delayed_work(&buf->work, 1);
    + goto out;
    + }
    +
    if (!test_and_set_bit(TTYP_FLUSHING, &port->iflags)) {
    struct tty_buffer *head;
    while ((head = buf->head) != NULL) {
    @@ -522,8 +529,11 @@ static void flush_to_ldisc(struct work_struct *work)
    if (test_bit(TTYP_FLUSHPENDING, &port->iflags)) {
    __tty_buffer_flush(port);
    clear_bit(TTYP_FLUSHPENDING, &port->iflags);
    - wake_up(&tty->read_wait);
    }
    +
    + /* wake up tasks waiting for TTYP_FLUSHING or TTYP_FLUSHPENDING */
    + wake_up_all(&tty->read_wait);
    +out:
    spin_unlock_irqrestore(&buf->lock, flags);

    tty_ldisc_deref(disc);
    diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
    index 8481b29..36a1bfa 100644
    --- a/drivers/tty/tty_ioctl.c
    +++ b/drivers/tty/tty_ioctl.c
    @@ -546,8 +546,33 @@ int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios)

    ld = tty_ldisc_ref(tty);
    if (ld != NULL) {
    - if (ld->ops->set_termios)
    + unsigned long flags;
    + struct tty_port *port = tty->port;
    + struct tty_bufhead *buf = &port->buf;
    + if (!ld->ops->set_termios)
    + goto no_set_termios;
    +
    + /* Wait if the data is being pushed to the tty layer */
    + spin_lock_irqsave(&buf->lock, flags);
    + while (test_bit(TTYP_FLUSHING, &port->iflags)) {
    + spin_unlock_irqrestore(&buf->lock, flags);
    + printk(KERN_CRIT "%s flushing\n", __FUNCTION__);
    + wait_event(tty->read_wait,
    + test_bit(TTYP_FLUSHING, &port->iflags) == 0);
    + spin_lock_irqsave(&buf->lock, flags);
    + }
    + printk(KERN_CRIT "%s survived flushing\n", __FUNCTION__);
    +
    + /* Prevent future flush_to_ldisc while we are setting */
    + if (!test_and_set_bit(TTY_LDISC_CHANGING, &tty->flags)) {
    + spin_unlock_irqrestore(&buf->lock, flags);
    (ld->ops->set_termios)(tty, &old_termios);
    + spin_lock_irqsave(&buf->lock, flags);
    + clear_bit(TTY_LDISC_CHANGING, &tty->flags);
    + }
    + spin_unlock_irqrestore(&buf->lock, flags);
    +
    +no_set_termios:
    tty_ldisc_deref(ld);
    }
    mutex_unlock(&tty->termios_mutex);
    --
    1.7.7.4


    \
     
     \ /
      Last update: 2013-02-23 04:21    [W:0.027 / U:32.752 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site