From patchwork Tue Feb 26 16:12:59 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Henriques X-Patchwork-Id: 223280 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id D03892C0082 for ; Wed, 27 Feb 2013 03:13:40 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1UAN9p-00017R-36; Tue, 26 Feb 2013 16:13:33 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1UAN9J-0000k0-FZ for kernel-team@lists.ubuntu.com; Tue, 26 Feb 2013 16:13:01 +0000 Received: from [188.251.62.197] (helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1UAN9I-0002Qk-Ly; Tue, 26 Feb 2013 16:13:01 +0000 From: Luis Henriques To: Dirkjan Bussink Subject: [ 3.5.y.z extended stable ] Patch "tty: Prevent deadlock in n_gsm driver" has been added to staging queue Date: Tue, 26 Feb 2013 16:12:59 +0000 Message-Id: <1361895179-27110-1-git-send-email-luis.henriques@canonical.com> X-Mailer: git-send-email 1.8.1.2 X-Extended-Stable: 3.5 Cc: Dirkjan Bussink , Greg Kroah-Hartman , kernel-team@lists.ubuntu.com X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com This is a note to let you know that I have just added a patch titled tty: Prevent deadlock in n_gsm driver to the linux-3.5.y-queue branch of the 3.5.y.z extended stable tree which can be found at: http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.5.y-queue If you, or anyone else, feels it should not be added to this tree, please reply to this email. For more information about the 3.5.y.z tree, see https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable Thanks. -Luis ------ From 53c4808395f551735fee16bdda701e7fe17c68aa Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Wed, 30 Jan 2013 11:44:50 +0100 Subject: [PATCH] tty: Prevent deadlock in n_gsm driver commit 4d9b109060f690f5c835130ff54165ae157b3087 upstream. This change fixes a deadlock when the multiplexer is closed while there are still client side ports open. When the multiplexer is closed and there are active tty's it tries to close them with tty_vhangup. This has a problem though, because tty_vhangup needs the tty_lock. This patch changes it to unlock the tty_lock before attempting the hangup and relocks afterwards. The additional call to tty_port_tty_set is needed because otherwise the port stays active because of the reference counter. This change also exposed another problem that other code paths don't expect that the multiplexer could have been closed. This patch also adds checks for these cases in the gsmtty_ class of function that could be called. The documentation explicitly states that "first close all virtual ports before closing the physical port" but we've found this to not always reality in our field situations. The GPRS / UTMS modem sometimes crashes and needs a power cycle in that case which means cleanly shutting down everything is not always possible. This change makes it much more robust for our situation where at least the system is recoverable with this patch and doesn't hang in a deadlock situation inside the kernel. The patch is against the long term support kernel (3.4.27) and should apply cleanly to more recent branches. Tested with a Telit GE864-QUADV2 and Telit HE910 modem. Signed-off-by: Dirkjan Bussink Signed-off-by: Greg Kroah-Hartman [ luis: adjust context ] Signed-off-by: Luis Henriques --- drivers/tty/n_gsm.c | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) -- 1.8.1.2 diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 90dff82..4056483 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -1692,6 +1692,8 @@ static inline void dlci_put(struct gsm_dlci *dlci) kref_put(&dlci->ref, gsm_dlci_free); } +static void gsm_destroy_network(struct gsm_dlci *dlci); + /** * gsm_dlci_release - release DLCI * @dlci: DLCI to destroy @@ -1705,9 +1707,19 @@ static void gsm_dlci_release(struct gsm_dlci *dlci) { struct tty_struct *tty = tty_port_tty_get(&dlci->port); if (tty) { + mutex_lock(&dlci->mutex); + gsm_destroy_network(dlci); + mutex_unlock(&dlci->mutex); + + /* tty_vhangup needs the tty_lock, so unlock and + relock after doing the hangup. */ + tty_unlock(tty); tty_vhangup(tty); + tty_lock(tty); + tty_port_tty_set(&dlci->port, NULL); tty_kref_put(tty); } + dlci->state = DLCI_CLOSED; dlci_put(dlci); } @@ -2933,6 +2945,8 @@ static void gsmtty_close(struct tty_struct *tty, struct file *filp) if (dlci == NULL) return; + if (dlci->state == DLCI_CLOSED) + return; mutex_lock(&dlci->mutex); gsm_destroy_network(dlci); mutex_unlock(&dlci->mutex); @@ -2951,6 +2965,8 @@ out: static void gsmtty_hangup(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return; tty_port_hangup(&dlci->port); gsm_dlci_begin_close(dlci); } @@ -2958,9 +2974,12 @@ static void gsmtty_hangup(struct tty_struct *tty) static int gsmtty_write(struct tty_struct *tty, const unsigned char *buf, int len) { + int sent; struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return -EINVAL; /* Stuff the bytes into the fifo queue */ - int sent = kfifo_in_locked(dlci->fifo, buf, len, &dlci->lock); + sent = kfifo_in_locked(dlci->fifo, buf, len, &dlci->lock); /* Need to kick the channel */ gsm_dlci_data_kick(dlci); return sent; @@ -2969,18 +2988,24 @@ static int gsmtty_write(struct tty_struct *tty, const unsigned char *buf, static int gsmtty_write_room(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return -EINVAL; return TX_SIZE - kfifo_len(dlci->fifo); } static int gsmtty_chars_in_buffer(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return -EINVAL; return kfifo_len(dlci->fifo); } static void gsmtty_flush_buffer(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return; /* Caution needed: If we implement reliable transport classes then the data being transmitted can't simply be junked once it has first hit the stack. Until then we can just blow it @@ -2999,6 +3024,8 @@ static void gsmtty_wait_until_sent(struct tty_struct *tty, int timeout) static int gsmtty_tiocmget(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return -EINVAL; return dlci->modem_rx; } @@ -3008,6 +3035,8 @@ static int gsmtty_tiocmset(struct tty_struct *tty, struct gsm_dlci *dlci = tty->driver_data; unsigned int modem_tx = dlci->modem_tx; + if (dlci->state == DLCI_CLOSED) + return -EINVAL; modem_tx &= ~clear; modem_tx |= set; @@ -3026,6 +3055,8 @@ static int gsmtty_ioctl(struct tty_struct *tty, struct gsm_netconfig nc; int index; + if (dlci->state == DLCI_CLOSED) + return -EINVAL; switch (cmd) { case GSMIOC_ENABLE_NET: if (copy_from_user(&nc, (void __user *)arg, sizeof(nc))) @@ -3052,6 +3083,9 @@ static int gsmtty_ioctl(struct tty_struct *tty, static void gsmtty_set_termios(struct tty_struct *tty, struct ktermios *old) { + struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return; /* For the moment its fixed. In actual fact the speed information for the virtual channel can be propogated in both directions by the RPN control message. This however rapidly gets nasty as we @@ -3063,6 +3097,8 @@ static void gsmtty_set_termios(struct tty_struct *tty, struct ktermios *old) static void gsmtty_throttle(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return; if (tty->termios->c_cflag & CRTSCTS) dlci->modem_tx &= ~TIOCM_DTR; dlci->throttled = 1; @@ -3073,6 +3109,8 @@ static void gsmtty_throttle(struct tty_struct *tty) static void gsmtty_unthrottle(struct tty_struct *tty) { struct gsm_dlci *dlci = tty->driver_data; + if (dlci->state == DLCI_CLOSED) + return; if (tty->termios->c_cflag & CRTSCTS) dlci->modem_tx |= TIOCM_DTR; dlci->throttled = 0; @@ -3084,6 +3122,8 @@ static int gsmtty_break_ctl(struct tty_struct *tty, int state) { struct gsm_dlci *dlci = tty->driver_data; int encode = 0; /* Off */ + if (dlci->state == DLCI_CLOSED) + return -EINVAL; if (state == -1) /* "On indefinitely" - we can't encode this properly */