linux-kernel - [PATCH] tty: n_gsm: do not send/receive in ldisc close path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Wed, 31 May 2017 08:19:05 +0200
From:   Sascha Hauer <s.hauer@...gutronix.de>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>, kernel@...gutronix.de,
        Jiri Slaby <jslaby@...e.com>,
        Sascha Hauer <s.hauer@...gutronix.de>
Subject: [PATCH] tty: n_gsm: do not send/receive in ldisc close path

gsm_cleanup_mux() is called in the line discipline close path which
is called at tty_release() time. At this stage the tty is no longer
operational enough to send any frames. Sending close frames is
therefore not possible and waiting for their answers always times
out.

This patch removes sending close messages and waiting for their answers
from the tty_release path.

This patch makes explicit what previously implicitly had been the case
already: We are not able to tell the modem that we are about to close
the multiplexer on our side. This means the modem will stay in
multiplexer mode and re-establishing the multiplexer later fails. The
only way for userspace to work around this is to manually send a close
frame in N_TTY mode after closing the mux.

Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
---

This is a resend of the same patch previously sent, but this time without
the second patch which was seen unnecessary.

 Documentation/serial/n_gsm.txt |  7 ++++++
 drivers/tty/n_gsm.c            | 54 +++++++++++++++++++++++++++---------------
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/Documentation/serial/n_gsm.txt b/Documentation/serial/n_gsm.txt
index a5d91126a8f7..875361bb7cb4 100644
--- a/Documentation/serial/n_gsm.txt
+++ b/Documentation/serial/n_gsm.txt
@@ -77,6 +77,13 @@ for example, it's possible :
 
 6- first close all virtual ports before closing the physical port.
 
+Note that after closing the physical port the modem is still in multiplexing
+mode. This may prevent a successful re-opening of the port later. To avoid
+this situation either reset the modem if your hardware allows that or send
+a disconnect command frame manually before initializing the multiplexing mode
+for the second time. The byte sequence for the disconnect command frame is:
+0xf9, 0x03, 0xef, 0x03, 0xc3, 0x16, 0xf9.
+
 Additional Documentation
 ------------------------
 More practical details on the protocol and how it's supported by industrial
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 2667a205a5ab..363a8ca824ad 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2015,6 +2015,33 @@ static void gsm_error(struct gsm_mux *gsm,
 	gsm->io_error++;
 }
 
+static int gsm_disconnect(struct gsm_mux *gsm)
+{
+	struct gsm_dlci *dlci = gsm->dlci[0];
+	struct gsm_control *gc;
+
+	if (!dlci)
+		return 0;
+
+	/* In theory disconnecting DLCI 0 is sufficient but for some
+	   modems this is apparently not the case. */
+	gc = gsm_control_send(gsm, CMD_CLD, NULL, 0);
+	if (gc)
+		gsm_control_wait(gsm, gc);
+
+	del_timer_sync(&gsm->t2_timer);
+	/* Now we are sure T2 has stopped */
+
+	gsm_dlci_begin_close(dlci);
+	wait_event_interruptible(gsm->event,
+				dlci->state == DLCI_CLOSED);
+
+	if (signal_pending(current))
+		return -EINTR;
+
+	return 0;
+}
+
 /**
  *	gsm_cleanup_mux		-	generic GSM protocol cleanup
  *	@gsm: our mux
@@ -2029,7 +2056,6 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm)
 	int i;
 	struct gsm_dlci *dlci = gsm->dlci[0];
 	struct gsm_msg *txq, *ntxq;
-	struct gsm_control *gc;
 
 	gsm->dead = 1;
 
@@ -2045,21 +2071,11 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm)
 	if (i == MAX_MUX)
 		return;
 
-	/* In theory disconnecting DLCI 0 is sufficient but for some
-	   modems this is apparently not the case. */
-	if (dlci) {
-		gc = gsm_control_send(gsm, CMD_CLD, NULL, 0);
-		if (gc)
-			gsm_control_wait(gsm, gc);
-	}
 	del_timer_sync(&gsm->t2_timer);
 	/* Now we are sure T2 has stopped */
-	if (dlci) {
+	if (dlci)
 		dlci->dead = 1;
-		gsm_dlci_begin_close(dlci);
-		wait_event_interruptible(gsm->event,
-					dlci->state == DLCI_CLOSED);
-	}
+
 	/* Free up any link layer users */
 	mutex_lock(&gsm->mutex);
 	for (i = 0; i < NUM_DLCI; i++)
@@ -2519,12 +2535,12 @@ static int gsmld_config(struct tty_struct *tty, struct gsm_mux *gsm,
 	 */
 
 	if (need_close || need_restart) {
-		gsm_dlci_begin_close(gsm->dlci[0]);
-		/* This will timeout if the link is down due to N2 expiring */
-		wait_event_interruptible(gsm->event,
-				gsm->dlci[0]->state == DLCI_CLOSED);
-		if (signal_pending(current))
-			return -EINTR;
+		int ret;
+
+		ret = gsm_disconnect(gsm);
+
+		if (ret)
+			return ret;
 	}
 	if (need_restart)
 		gsm_cleanup_mux(gsm);
-- 
2.11.0