tty: Serialize tty flow control changes with flow_lock
authorPeter Hurley <peter@hurleysoftware.com>
Wed, 10 Sep 2014 19:06:31 +0000 (15:06 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 24 Sep 2014 04:19:35 +0000 (21:19 -0700)
Without serialization, the flow control state can become inverted
wrt. the actual hardware state. For example,

CPU 0                          | CPU 1
stop_tty()                     |
  lock ctrl_lock               |
  tty->stopped = 1             |
  unlock ctrl_lock             |
                               | start_tty()
                               |   lock ctrl_lock
                               |   tty->stopped = 0
                               |   unlock ctrl_lock
                               |   driver->start()
  driver->stop()               |

In this case, the flow control state now indicates the tty has
been started, but the actual hardware state has actually been stopped.

Introduce tty->flow_lock spinlock to serialize tty flow control changes.
Split out unlocked __start_tty()/__stop_tty() flavors for use by
ioctl(TCXONC) in follow-on patch.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/tty_io.c
include/linux/tty.h
include/linux/tty_driver.h

index d4eb2a8b70476b5b349ef40c4a43f2bddad0604b..b8ddfef6b5d8d0cb4e2a21e1632cfece69e6d2a5 100644 (file)
@@ -919,18 +919,18 @@ void no_tty(void)
  *     but not always.
  *
  *     Locking:
- *             Uses the tty control lock internally
+ *             ctrl_lock
+ *             flow_lock
  */
 
-void stop_tty(struct tty_struct *tty)
+void __stop_tty(struct tty_struct *tty)
 {
        unsigned long flags;
-       spin_lock_irqsave(&tty->ctrl_lock, flags);
-       if (tty->stopped) {
-               spin_unlock_irqrestore(&tty->ctrl_lock, flags);
+
+       if (tty->stopped)
                return;
-       }
        tty->stopped = 1;
+       spin_lock_irqsave(&tty->ctrl_lock, flags);
        if (tty->link && tty->link->packet) {
                tty->ctrl_status &= ~TIOCPKT_START;
                tty->ctrl_status |= TIOCPKT_STOP;
@@ -941,6 +941,14 @@ void stop_tty(struct tty_struct *tty)
                (tty->ops->stop)(tty);
 }
 
+void stop_tty(struct tty_struct *tty)
+{
+       unsigned long flags;
+
+       spin_lock_irqsave(&tty->flow_lock, flags);
+       __stop_tty(tty);
+       spin_unlock_irqrestore(&tty->flow_lock, flags);
+}
 EXPORT_SYMBOL(stop_tty);
 
 /**
@@ -954,17 +962,17 @@ EXPORT_SYMBOL(stop_tty);
  *
  *     Locking:
  *             ctrl_lock
+ *             flow_lock
  */
 
-void start_tty(struct tty_struct *tty)
+void __start_tty(struct tty_struct *tty)
 {
        unsigned long flags;
-       spin_lock_irqsave(&tty->ctrl_lock, flags);
-       if (!tty->stopped || tty->flow_stopped) {
-               spin_unlock_irqrestore(&tty->ctrl_lock, flags);
+
+       if (!tty->stopped || tty->flow_stopped)
                return;
-       }
        tty->stopped = 0;
+       spin_lock_irqsave(&tty->ctrl_lock, flags);
        if (tty->link && tty->link->packet) {
                tty->ctrl_status &= ~TIOCPKT_STOP;
                tty->ctrl_status |= TIOCPKT_START;
@@ -977,6 +985,14 @@ void start_tty(struct tty_struct *tty)
        tty_wakeup(tty);
 }
 
+void start_tty(struct tty_struct *tty)
+{
+       unsigned long flags;
+
+       spin_lock_irqsave(&tty->flow_lock, flags);
+       __start_tty(tty);
+       spin_unlock_irqrestore(&tty->flow_lock, flags);
+}
 EXPORT_SYMBOL(start_tty);
 
 /* We limit tty time update visibility to every 8 seconds or so. */
@@ -3019,6 +3035,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
        INIT_WORK(&tty->hangup_work, do_tty_hangup);
        mutex_init(&tty->atomic_write_lock);
        spin_lock_init(&tty->ctrl_lock);
+       spin_lock_init(&tty->flow_lock);
        INIT_LIST_HEAD(&tty->tty_files);
        INIT_WORK(&tty->SAK_work, do_SAK_work);
 
index 4cfd4a82fc31a9ab01253b8cd0401be8dd0962ab..fd4148d3a261dbc96f2ccc4f1a0541319f50b691 100644 (file)
@@ -252,6 +252,7 @@ struct tty_struct {
        struct rw_semaphore termios_rwsem;
        struct mutex winsize_mutex;
        spinlock_t ctrl_lock;
+       spinlock_t flow_lock;
        /* Termios values are protected by the termios rwsem */
        struct ktermios termios, termios_locked;
        struct termiox *termiox;        /* May be NULL for unsupported */
@@ -261,7 +262,7 @@ struct tty_struct {
        unsigned long flags;
        int count;
        struct winsize winsize;         /* winsize_mutex */
-       int stopped;
+       int stopped;                    /* flow_lock */
        int flow_stopped;
        int hw_stopped;
        int packet;
@@ -400,7 +401,9 @@ extern int tty_paranoia_check(struct tty_struct *tty, struct inode *inode,
 extern char *tty_name(struct tty_struct *tty, char *buf);
 extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
 extern int tty_check_change(struct tty_struct *tty);
+extern void __stop_tty(struct tty_struct *tty);
 extern void stop_tty(struct tty_struct *tty);
+extern void __start_tty(struct tty_struct *tty);
 extern void start_tty(struct tty_struct *tty);
 extern int tty_register_driver(struct tty_driver *driver);
 extern int tty_unregister_driver(struct tty_driver *driver);
index e48c608a8fa8b66e60f5e9fe4efe6f0b4616a6ee..92e337c18839b0df2e9c213fd0604983dfcdaefd 100644 (file)
  *     This routine notifies the tty driver that it should stop
  *     outputting characters to the tty device.  
  *
+ *     Called with ->flow_lock held. Serialized with start() method.
+ *
  *     Optional:
  *
  *     Note: Call stop_tty not this method.
  *     This routine notifies the tty driver that it resume sending
  *     characters to the tty device.
  *
+ *     Called with ->flow_lock held. Serialized with stop() method.
+ *
  *     Optional:
  *
  *     Note: Call start_tty not this method.