tty: Fix recursive deadlock in tty_perform_flush()
authorPeter Hurley <peter@hurleysoftware.com>
Mon, 11 Mar 2013 20:44:45 +0000 (16:44 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 18 Mar 2013 23:52:24 +0000 (16:52 -0700)
tty_perform_flush() can deadlock when called while holding
a line discipline reference. By definition, all ldisc drivers
hold a ldisc reference, so calls originating from ldisc drivers
must not block for a ldisc reference.

The deadlock can occur when:
  CPU 0                    |  CPU 1
                           |
tty_ldisc_ref(tty)         |
....                       | <line discipline halted>
tty_ldisc_ref_wait(tty)    |
                           |

CPU 0 cannot progess because it cannot obtain an ldisc reference
with the line discipline has been halted (thus no new references
are granted).
CPU 1 cannot progress because an outstanding ldisc reference
has not been released.

An in-tree call-tree audit of tty_perform_flush() [1] shows 5
ldisc drivers calling tty_perform_flush() indirectly via
n_tty_ioctl_helper() and 2 ldisc drivers calling directly.
A single tty driver safely uses the function.

[1]
Recursive usage:

/* These functions are line discipline ioctls and thus
 * recursive wrt line discipline references */

tty_perform_flush() - ./drivers/tty/tty_ioctl.c
    n_tty_ioctl_helper()
        hci_uart_tty_ioctl(default) - drivers/bluetooth/hci_ldisc.c (N_HCI)
        n_hdlc_tty_ioctl(default) - drivers/tty/n_hdlc.c (N_HDLC)
        gsmld_ioctl(default) - drivers/tty/n_gsm.c (N_GSM0710)
        n_tty_ioctl(default) - drivers/tty/n_tty.c (N_TTY)
        gigaset_tty_ioctl(default) - drivers/isdn/gigaset/ser-gigaset.c (N_GIGASET_M101)
    ppp_synctty_ioctl(TCFLSH) - drivers/net/ppp/pps_synctty.c
    ppp_asynctty_ioctl(TCFLSH) - drivers/net/ppp/ppp_async.c

Non-recursive use:

tty_perform_flush() - drivers/tty/tty_ioctl.c
    ipw_ioctl(TCFLSH) - drivers/tty/ipwireless/tty.c
       /* This function is a tty i/o ioctl method, which
        * is invoked by tty_ioctl() */

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/net/ppp/ppp_async.c
drivers/net/ppp/ppp_synctty.c
drivers/tty/tty_ioctl.c

index a031f6b456b4d90d0d356bc49ad330eca6570c1b..9c889e0303dd1f079c301c9a008fa8a0a094e90b 100644 (file)
@@ -314,7 +314,7 @@ ppp_asynctty_ioctl(struct tty_struct *tty, struct file *file,
                /* flush our buffers and the serial port's buffer */
                if (arg == TCIOFLUSH || arg == TCOFLUSH)
                        ppp_async_flush_output(ap);
-               err = tty_perform_flush(tty, arg);
+               err = n_tty_ioctl_helper(tty, file, cmd, arg);
                break;
 
        case FIONREAD:
index 1a12033d2efafa83d2d118ead1a80262f961a441..bdf3b13a71a83991117b1d843e77f1de2d1aa415 100644 (file)
@@ -355,7 +355,7 @@ ppp_synctty_ioctl(struct tty_struct *tty, struct file *file,
                /* flush our buffers and the serial port's buffer */
                if (arg == TCIOFLUSH || arg == TCOFLUSH)
                        ppp_sync_flush_output(ap);
-               err = tty_perform_flush(tty, arg);
+               err = n_tty_ioctl_helper(tty, file, cmd, arg);
                break;
 
        case FIONREAD:
index 28715e48b2f771907498be99ef266097f54312e4..d119034877decb4402c1f2bad5b87e896a20943e 100644 (file)
@@ -1122,14 +1122,12 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
 }
 EXPORT_SYMBOL_GPL(tty_mode_ioctl);
 
-int tty_perform_flush(struct tty_struct *tty, unsigned long arg)
+
+/* Caller guarantees ldisc reference is held */
+static int __tty_perform_flush(struct tty_struct *tty, unsigned long arg)
 {
-       struct tty_ldisc *ld;
-       int retval = tty_check_change(tty);
-       if (retval)
-               return retval;
+       struct tty_ldisc *ld = tty->ldisc;
 
-       ld = tty_ldisc_ref_wait(tty);
        switch (arg) {
        case TCIFLUSH:
                if (ld && ld->ops->flush_buffer) {
@@ -1147,12 +1145,24 @@ int tty_perform_flush(struct tty_struct *tty, unsigned long arg)
                tty_driver_flush_buffer(tty);
                break;
        default:
-               tty_ldisc_deref(ld);
                return -EINVAL;
        }
-       tty_ldisc_deref(ld);
        return 0;
 }
+
+int tty_perform_flush(struct tty_struct *tty, unsigned long arg)
+{
+       struct tty_ldisc *ld;
+       int retval = tty_check_change(tty);
+       if (retval)
+               return retval;
+
+       ld = tty_ldisc_ref_wait(tty);
+       retval = __tty_perform_flush(tty, arg);
+       if (ld)
+               tty_ldisc_deref(ld);
+       return retval;
+}
 EXPORT_SYMBOL_GPL(tty_perform_flush);
 
 int n_tty_ioctl_helper(struct tty_struct *tty, struct file *file,
@@ -1191,7 +1201,7 @@ int n_tty_ioctl_helper(struct tty_struct *tty, struct file *file,
                }
                return 0;
        case TCFLSH:
-               return tty_perform_flush(tty, arg);
+               return __tty_perform_flush(tty, arg);
        default:
                /* Try the mode commands */
                return tty_mode_ioctl(tty, file, cmd, arg);