[PATCH 018/139] tty: Prevent deadlock in n_gsm driver

Philip Müller philm at manjaro.org
Thu Feb 28 20:00:15 UTC 2013


Hi Luis,

this patch creates followed error:

CC [M] drivers/staging/gdm72xx/usb_boot.o
CC [M] drivers/scsi/pm8001/pm8001_ctl.o
CC [M] drivers/uio/uio_pci_generic.o
LD [M] drivers/staging/gdm72xx/gdmwm.o
LD drivers/staging/ipack/bridges/built-in.o
CC [M] drivers/tty/n_gsm.o
CC [M] drivers/staging/ipack/bridges/tpci200.o
CC [M] drivers/uio/uio_netx.o
CC [M] drivers/scsi/pm8001/pm8001_hwi.o
LD [M] drivers/net/wireless/rtlwifi/rtl8192c/rtl8192c-common.o
drivers/tty/n_gsm.c: In function ‘gsm_dlci_release’:
drivers/tty/n_gsm.c:1716:3: error: too many arguments to function 
‘tty_unlock’
In file included from drivers/tty/n_gsm.c:44:0:
include/linux/tty.h:609:24: note: declared here
drivers/tty/n_gsm.c:1718:3: error: too many arguments to function ‘tty_lock’
In file included from drivers/tty/n_gsm.c:44:0:
include/linux/tty.h:608:24: note: declared here
make[2]: *** [drivers/tty/n_gsm.o] Error 1
make[1]: *** [drivers/tty] Error 2
make[1]: *** Waiting for unfinished jobs....
LD drivers/net/wireless/rtlwifi/rtl8192ce/built-in.o
CC [M] drivers/net/wireless/rtlwifi/rtl8192ce/dm.o
CC [M] drivers/net/tun.o
CC [M] drivers/net/wireless/rtlwifi/rtl8192ce/hw.o
LD drivers/staging/ipack/devices/built-in.o

Modified n_gsm.c file is attached, which creates the error.

kind regards

Philip Müller

Am 28.02.2013 15:42, schrieb Luis Henriques:
> 3.5.7.7 -stable review patch.  If anyone has any objections, please let me know.
>
> ------------------
>
> From: Dirkjan Bussink <d.bussink at gmail.com>
>
> 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 <dirkjan.bussink at nedap.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> [ luis: adjust context ]
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>   drivers/tty/n_gsm.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 41 insertions(+), 1 deletion(-)
>
> 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 */

-------------- next part --------------
A non-text attachment was scrubbed...
Name: n_gsm.c
Type: text/x-csrc
Size: 80180 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20130228/62a8a5fe/attachment.c>


More information about the kernel-team mailing list