diff mbox series

[5.17,219/225] tty: n_gsm: fix broken virtual tty handling

Message ID 20220504153129.436268194@linuxfoundation.org
State New
Headers show
Series None | expand

Commit Message

Greg KH May 4, 2022, 4:47 p.m. UTC 
From: Daniel Starke <daniel.starke@siemens.com>

commit a8c5b8255f8a9acd58a4b15ff1c14cd6effd114b upstream.

Dynamic virtual tty registration was introduced to allow the user to handle
these cases with uevent rules. The following commits relate to this:
Commit 5b87686e3203 ("tty: n_gsm: Modify gsmtty driver register method when config requester")
Commit 0b91b5332368 ("tty: n_gsm: Save dlci address open status when config requester")
Commit 46292622ad73 ("tty: n_gsm: clean up indenting in gsm_queue()")

However, the following behavior can be seen with this implementation:
- n_gsm ldisc is activated via ioctl
- all configuration parameters are set to their default value (initiator=0)
- the mux gets activated and attached and gsmtty0 is being registered in
  in gsm_dlci_open() after DLCI 0 was established (DLCI 0 is the control
  channel)
- the user configures n_gsm via ioctl GSMIOC_SETCONF as initiator
- this re-attaches the n_gsm mux
- no new gsmtty devices are registered in gsmld_attach_gsm() because the
  mux is already active
- the initiator side registered only the control channel as gsmtty0
  (which should never happen) and no user channel tty

The commits above make it impossible to operate the initiator side as no
user channel tty is or will be available.
On the other hand, this behavior will make it also impossible to allow DLCI
parameter negotiation on responder side in the future. The responder side
first needs to provide a device for the application before the application
can set its parameters of the associated DLCI via ioctl.
Note that the user application is still able to detect a link establishment
without relaying to uevent by waiting for DTR open on responder side. This
is the same behavior as on a physical serial interface. And on initiator
side a tty hangup can be detected if a link establishment request failed.

Revert the commits above completely to always register all user channels
and no control channel after mux attachment. No other changes are made.

Fixes: 5b87686e3203 ("tty: n_gsm: Modify gsmtty driver register method when config requester")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
Link: https://lore.kernel.org/r/20220422071025.5490-1-daniel.starke@siemens.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/n_gsm.c |   87 ++++++++--------------------------------------------
 1 file changed, 15 insertions(+), 72 deletions(-)
diff mbox series

Patch

--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -272,10 +272,6 @@  static DEFINE_SPINLOCK(gsm_mux_lock);
 
 static struct tty_driver *gsm_tty_driver;
 
-/* Save dlci open address */
-static int addr_open[256] = { 0 };
-/* Save dlci open count */
-static int addr_cnt;
 /*
  *	This section of the driver logic implements the GSM encodings
  *	both the basic and the 'advanced'. Reliable transport is not
@@ -1185,7 +1181,6 @@  static void gsm_control_rls(struct gsm_m
 }
 
 static void gsm_dlci_begin_close(struct gsm_dlci *dlci);
-static void gsm_dlci_close(struct gsm_dlci *dlci);
 
 /**
  *	gsm_control_message	-	DLCI 0 control processing
@@ -1204,28 +1199,15 @@  static void gsm_control_message(struct g
 {
 	u8 buf[1];
 	unsigned long flags;
-	struct gsm_dlci *dlci;
-	int i;
-	int address;
 
 	switch (command) {
 	case CMD_CLD: {
-		if (addr_cnt > 0) {
-			for (i = 0; i < addr_cnt; i++) {
-				address = addr_open[i];
-				dlci = gsm->dlci[address];
-				gsm_dlci_close(dlci);
-				addr_open[i] = 0;
-			}
-		}
+		struct gsm_dlci *dlci = gsm->dlci[0];
 		/* Modem wishes to close down */
-		dlci = gsm->dlci[0];
 		if (dlci) {
 			dlci->dead = true;
 			gsm->dead = true;
-			gsm_dlci_close(dlci);
-			addr_cnt = 0;
-			gsm_response(gsm, 0, UA|PF);
+			gsm_dlci_begin_close(dlci);
 		}
 		}
 		break;
@@ -1459,8 +1441,6 @@  static void gsm_dlci_close(struct gsm_dl
 		wake_up_interruptible(&dlci->port.open_wait);
 	} else
 		dlci->gsm->dead = true;
-	/* Unregister gsmtty driver,report gsmtty dev remove uevent for user */
-	tty_unregister_device(gsm_tty_driver, dlci->addr);
 	wake_up(&dlci->gsm->event);
 	/* A DLCI 0 close is a MUX termination so we need to kick that
 	   back to userspace somehow */
@@ -1482,8 +1462,6 @@  static void gsm_dlci_open(struct gsm_dlc
 	dlci->state = DLCI_OPEN;
 	if (debug & 8)
 		pr_debug("DLCI %d goes open.\n", dlci->addr);
-	/* Register gsmtty driver,report gsmtty dev add uevent for user */
-	tty_register_device(gsm_tty_driver, dlci->addr, NULL);
 	/* Send current modem state */
 	if (dlci->addr)
 		gsmtty_modem_update(dlci, 0);
@@ -1794,7 +1772,6 @@  static void gsm_queue(struct gsm_mux *gs
 	struct gsm_dlci *dlci;
 	u8 cr;
 	int address;
-	int i, j, k, address_tmp;
 
 	if (gsm->fcs != GOOD_FCS) {
 		gsm->bad_fcs++;
@@ -1826,11 +1803,6 @@  static void gsm_queue(struct gsm_mux *gs
 		else {
 			gsm_response(gsm, address, UA|PF);
 			gsm_dlci_open(dlci);
-			/* Save dlci open address */
-			if (address) {
-				addr_open[addr_cnt] = address;
-				addr_cnt++;
-			}
 		}
 		break;
 	case DISC|PF:
@@ -1841,33 +1813,8 @@  static void gsm_queue(struct gsm_mux *gs
 			return;
 		}
 		/* Real close complete */
-		if (!address) {
-			if (addr_cnt > 0) {
-				for (i = 0; i < addr_cnt; i++) {
-					address = addr_open[i];
-					dlci = gsm->dlci[address];
-					gsm_dlci_close(dlci);
-					addr_open[i] = 0;
-				}
-			}
-			dlci = gsm->dlci[0];
-			gsm_dlci_close(dlci);
-			addr_cnt = 0;
-			gsm_response(gsm, 0, UA|PF);
-		} else {
-			gsm_response(gsm, address, UA|PF);
-			gsm_dlci_close(dlci);
-			/* clear dlci address */
-			for (j = 0; j < addr_cnt; j++) {
-				address_tmp = addr_open[j];
-				if (address_tmp == address) {
-					for (k = j; k < addr_cnt; k++)
-						addr_open[k] = addr_open[k+1];
-					addr_cnt--;
-					break;
-				}
-			}
-		}
+		gsm_response(gsm, address, UA|PF);
+		gsm_dlci_close(dlci);
 		break;
 	case UA|PF:
 		if (cr == 0 || dlci == NULL)
@@ -2451,19 +2398,17 @@  static int gsmld_attach_gsm(struct tty_s
 	else {
 		/* Don't register device 0 - this is the control channel and not
 		   a usable tty interface */
-		if (gsm->initiator) {
-			base = mux_num_to_base(gsm); /* Base for this MUX */
-			for (i = 1; i < NUM_DLCI; i++) {
-				struct device *dev;
+		base = mux_num_to_base(gsm); /* Base for this MUX */
+		for (i = 1; i < NUM_DLCI; i++) {
+			struct device *dev;
 
-				dev = tty_register_device(gsm_tty_driver,
+			dev = tty_register_device(gsm_tty_driver,
 							base + i, NULL);
-				if (IS_ERR(dev)) {
-					for (i--; i >= 1; i--)
-						tty_unregister_device(gsm_tty_driver,
-									base + i);
-					return PTR_ERR(dev);
-				}
+			if (IS_ERR(dev)) {
+				for (i--; i >= 1; i--)
+					tty_unregister_device(gsm_tty_driver,
+								base + i);
+				return PTR_ERR(dev);
 			}
 		}
 	}
@@ -2485,10 +2430,8 @@  static void gsmld_detach_gsm(struct tty_
 	int i;
 
 	WARN_ON(tty != gsm->tty);
-	if (gsm->initiator) {
-		for (i = 1; i < NUM_DLCI; i++)
-			tty_unregister_device(gsm_tty_driver, base + i);
-	}
+	for (i = 1; i < NUM_DLCI; i++)
+		tty_unregister_device(gsm_tty_driver, base + i);
 	tty_kref_put(gsm->tty);
 	gsm->tty = NULL;
 }