Skip to content

Commit 4d9b109

Browse files
dbussinkgregkh
authored andcommitted
tty: Prevent deadlock in n_gsm driver
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 <dirkjan.bussink@nedap.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent c420811 commit 4d9b109

File tree

1 file changed

+41
-1
lines changed

1 file changed

+41
-1
lines changed

drivers/tty/n_gsm.c

+41-1
Original file line numberDiff line numberDiff line change
@@ -1689,6 +1689,8 @@ static inline void dlci_put(struct gsm_dlci *dlci)
16891689
tty_port_put(&dlci->port);
16901690
}
16911691

1692+
static void gsm_destroy_network(struct gsm_dlci *dlci);
1693+
16921694
/**
16931695
* gsm_dlci_release - release DLCI
16941696
* @dlci: DLCI to destroy
@@ -1702,9 +1704,19 @@ static void gsm_dlci_release(struct gsm_dlci *dlci)
17021704
{
17031705
struct tty_struct *tty = tty_port_tty_get(&dlci->port);
17041706
if (tty) {
1707+
mutex_lock(&dlci->mutex);
1708+
gsm_destroy_network(dlci);
1709+
mutex_unlock(&dlci->mutex);
1710+
1711+
/* tty_vhangup needs the tty_lock, so unlock and
1712+
relock after doing the hangup. */
1713+
tty_unlock(tty);
17051714
tty_vhangup(tty);
1715+
tty_lock(tty);
1716+
tty_port_tty_set(&dlci->port, NULL);
17061717
tty_kref_put(tty);
17071718
}
1719+
dlci->state = DLCI_CLOSED;
17081720
dlci_put(dlci);
17091721
}
17101722

@@ -2947,6 +2959,8 @@ static void gsmtty_close(struct tty_struct *tty, struct file *filp)
29472959

29482960
if (dlci == NULL)
29492961
return;
2962+
if (dlci->state == DLCI_CLOSED)
2963+
return;
29502964
mutex_lock(&dlci->mutex);
29512965
gsm_destroy_network(dlci);
29522966
mutex_unlock(&dlci->mutex);
@@ -2965,16 +2979,21 @@ static void gsmtty_close(struct tty_struct *tty, struct file *filp)
29652979
static void gsmtty_hangup(struct tty_struct *tty)
29662980
{
29672981
struct gsm_dlci *dlci = tty->driver_data;
2982+
if (dlci->state == DLCI_CLOSED)
2983+
return;
29682984
tty_port_hangup(&dlci->port);
29692985
gsm_dlci_begin_close(dlci);
29702986
}
29712987

29722988
static int gsmtty_write(struct tty_struct *tty, const unsigned char *buf,
29732989
int len)
29742990
{
2991+
int sent;
29752992
struct gsm_dlci *dlci = tty->driver_data;
2993+
if (dlci->state == DLCI_CLOSED)
2994+
return -EINVAL;
29762995
/* Stuff the bytes into the fifo queue */
2977-
int sent = kfifo_in_locked(dlci->fifo, buf, len, &dlci->lock);
2996+
sent = kfifo_in_locked(dlci->fifo, buf, len, &dlci->lock);
29782997
/* Need to kick the channel */
29792998
gsm_dlci_data_kick(dlci);
29802999
return sent;
@@ -2983,18 +3002,24 @@ static int gsmtty_write(struct tty_struct *tty, const unsigned char *buf,
29833002
static int gsmtty_write_room(struct tty_struct *tty)
29843003
{
29853004
struct gsm_dlci *dlci = tty->driver_data;
3005+
if (dlci->state == DLCI_CLOSED)
3006+
return -EINVAL;
29863007
return TX_SIZE - kfifo_len(dlci->fifo);
29873008
}
29883009

29893010
static int gsmtty_chars_in_buffer(struct tty_struct *tty)
29903011
{
29913012
struct gsm_dlci *dlci = tty->driver_data;
3013+
if (dlci->state == DLCI_CLOSED)
3014+
return -EINVAL;
29923015
return kfifo_len(dlci->fifo);
29933016
}
29943017

29953018
static void gsmtty_flush_buffer(struct tty_struct *tty)
29963019
{
29973020
struct gsm_dlci *dlci = tty->driver_data;
3021+
if (dlci->state == DLCI_CLOSED)
3022+
return;
29983023
/* Caution needed: If we implement reliable transport classes
29993024
then the data being transmitted can't simply be junked once
30003025
it has first hit the stack. Until then we can just blow it
@@ -3013,6 +3038,8 @@ static void gsmtty_wait_until_sent(struct tty_struct *tty, int timeout)
30133038
static int gsmtty_tiocmget(struct tty_struct *tty)
30143039
{
30153040
struct gsm_dlci *dlci = tty->driver_data;
3041+
if (dlci->state == DLCI_CLOSED)
3042+
return -EINVAL;
30163043
return dlci->modem_rx;
30173044
}
30183045

@@ -3022,6 +3049,8 @@ static int gsmtty_tiocmset(struct tty_struct *tty,
30223049
struct gsm_dlci *dlci = tty->driver_data;
30233050
unsigned int modem_tx = dlci->modem_tx;
30243051

3052+
if (dlci->state == DLCI_CLOSED)
3053+
return -EINVAL;
30253054
modem_tx &= ~clear;
30263055
modem_tx |= set;
30273056

@@ -3040,6 +3069,8 @@ static int gsmtty_ioctl(struct tty_struct *tty,
30403069
struct gsm_netconfig nc;
30413070
int index;
30423071

3072+
if (dlci->state == DLCI_CLOSED)
3073+
return -EINVAL;
30433074
switch (cmd) {
30443075
case GSMIOC_ENABLE_NET:
30453076
if (copy_from_user(&nc, (void __user *)arg, sizeof(nc)))
@@ -3066,6 +3097,9 @@ static int gsmtty_ioctl(struct tty_struct *tty,
30663097

30673098
static void gsmtty_set_termios(struct tty_struct *tty, struct ktermios *old)
30683099
{
3100+
struct gsm_dlci *dlci = tty->driver_data;
3101+
if (dlci->state == DLCI_CLOSED)
3102+
return;
30693103
/* For the moment its fixed. In actual fact the speed information
30703104
for the virtual channel can be propogated in both directions by
30713105
the RPN control message. This however rapidly gets nasty as we
@@ -3077,6 +3111,8 @@ static void gsmtty_set_termios(struct tty_struct *tty, struct ktermios *old)
30773111
static void gsmtty_throttle(struct tty_struct *tty)
30783112
{
30793113
struct gsm_dlci *dlci = tty->driver_data;
3114+
if (dlci->state == DLCI_CLOSED)
3115+
return;
30803116
if (tty->termios.c_cflag & CRTSCTS)
30813117
dlci->modem_tx &= ~TIOCM_DTR;
30823118
dlci->throttled = 1;
@@ -3087,6 +3123,8 @@ static void gsmtty_throttle(struct tty_struct *tty)
30873123
static void gsmtty_unthrottle(struct tty_struct *tty)
30883124
{
30893125
struct gsm_dlci *dlci = tty->driver_data;
3126+
if (dlci->state == DLCI_CLOSED)
3127+
return;
30903128
if (tty->termios.c_cflag & CRTSCTS)
30913129
dlci->modem_tx |= TIOCM_DTR;
30923130
dlci->throttled = 0;
@@ -3098,6 +3136,8 @@ static int gsmtty_break_ctl(struct tty_struct *tty, int state)
30983136
{
30993137
struct gsm_dlci *dlci = tty->driver_data;
31003138
int encode = 0; /* Off */
3139+
if (dlci->state == DLCI_CLOSED)
3140+
return -EINVAL;
31013141

31023142
if (state == -1) /* "On indefinitely" - we can't encode this
31033143
properly */

0 commit comments

Comments
 (0)