[PATCH v5 25/44] tty: Fix recursive deadlock in tty_perform_flush()

From: Peter Hurley
Date: Mon Mar 11 2013 - 17:27:48 EST


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@xxxxxxxxxxxxxxxxxx>
---
drivers/net/ppp/ppp_async.c | 2 +-
drivers/net/ppp/ppp_synctty.c | 2 +-
drivers/tty/tty_ioctl.c | 28 +++++++++++++++++++---------
3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
index a031f6b..9c889e0 100644
--- a/drivers/net/ppp/ppp_async.c
+++ b/drivers/net/ppp/ppp_async.c
@@ -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:
diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
index 1a12033..bdf3b13 100644
--- a/drivers/net/ppp/ppp_synctty.c
+++ b/drivers/net/ppp/ppp_synctty.c
@@ -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:
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index d58b92c..935b032 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -1086,14 +1086,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) {
@@ -1111,12 +1109,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,
@@ -1155,7 +1165,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);
--
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/