Skip to content

Commit a8c5b82

Browse files
dstarke-siemensgregkh
authored andcommitted
tty: n_gsm: fix broken virtual tty handling
Dynamic virtual tty registration was introduced to allow the user to handle these cases with uevent rules. The following commits relate to this: Commit 5b87686 ("tty: n_gsm: Modify gsmtty driver register method when config requester") Commit 0b91b53 ("tty: n_gsm: Save dlci address open status when config requester") Commit 4629262 ("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: 5b87686 ("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>
1 parent 41c6068 commit a8c5b82

File tree

1 file changed

+15
-72
lines changed

1 file changed

+15
-72
lines changed

drivers/tty/n_gsm.c

Lines changed: 15 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,6 @@ static DEFINE_SPINLOCK(gsm_mux_lock);
272272

273273
static struct tty_driver *gsm_tty_driver;
274274

275-
/* Save dlci open address */
276-
static int addr_open[256] = { 0 };
277-
/* Save dlci open count */
278-
static int addr_cnt;
279275
/*
280276
* This section of the driver logic implements the GSM encodings
281277
* both the basic and the 'advanced'. Reliable transport is not
@@ -1185,7 +1181,6 @@ static void gsm_control_rls(struct gsm_mux *gsm, const u8 *data, int clen)
11851181
}
11861182

11871183
static void gsm_dlci_begin_close(struct gsm_dlci *dlci);
1188-
static void gsm_dlci_close(struct gsm_dlci *dlci);
11891184

11901185
/**
11911186
* gsm_control_message - DLCI 0 control processing
@@ -1204,28 +1199,15 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command,
12041199
{
12051200
u8 buf[1];
12061201
unsigned long flags;
1207-
struct gsm_dlci *dlci;
1208-
int i;
1209-
int address;
12101202

12111203
switch (command) {
12121204
case CMD_CLD: {
1213-
if (addr_cnt > 0) {
1214-
for (i = 0; i < addr_cnt; i++) {
1215-
address = addr_open[i];
1216-
dlci = gsm->dlci[address];
1217-
gsm_dlci_close(dlci);
1218-
addr_open[i] = 0;
1219-
}
1220-
}
1205+
struct gsm_dlci *dlci = gsm->dlci[0];
12211206
/* Modem wishes to close down */
1222-
dlci = gsm->dlci[0];
12231207
if (dlci) {
12241208
dlci->dead = true;
12251209
gsm->dead = true;
1226-
gsm_dlci_close(dlci);
1227-
addr_cnt = 0;
1228-
gsm_response(gsm, 0, UA|PF);
1210+
gsm_dlci_begin_close(dlci);
12291211
}
12301212
}
12311213
break;
@@ -1459,8 +1441,6 @@ static void gsm_dlci_close(struct gsm_dlci *dlci)
14591441
wake_up_interruptible(&dlci->port.open_wait);
14601442
} else
14611443
dlci->gsm->dead = true;
1462-
/* Unregister gsmtty driver,report gsmtty dev remove uevent for user */
1463-
tty_unregister_device(gsm_tty_driver, dlci->addr);
14641444
wake_up(&dlci->gsm->event);
14651445
/* A DLCI 0 close is a MUX termination so we need to kick that
14661446
back to userspace somehow */
@@ -1482,8 +1462,6 @@ static void gsm_dlci_open(struct gsm_dlci *dlci)
14821462
dlci->state = DLCI_OPEN;
14831463
if (debug & 8)
14841464
pr_debug("DLCI %d goes open.\n", dlci->addr);
1485-
/* Register gsmtty driver,report gsmtty dev add uevent for user */
1486-
tty_register_device(gsm_tty_driver, dlci->addr, NULL);
14871465
/* Send current modem state */
14881466
if (dlci->addr)
14891467
gsmtty_modem_update(dlci, 0);
@@ -1794,7 +1772,6 @@ static void gsm_queue(struct gsm_mux *gsm)
17941772
struct gsm_dlci *dlci;
17951773
u8 cr;
17961774
int address;
1797-
int i, j, k, address_tmp;
17981775

17991776
if (gsm->fcs != GOOD_FCS) {
18001777
gsm->bad_fcs++;
@@ -1826,11 +1803,6 @@ static void gsm_queue(struct gsm_mux *gsm)
18261803
else {
18271804
gsm_response(gsm, address, UA|PF);
18281805
gsm_dlci_open(dlci);
1829-
/* Save dlci open address */
1830-
if (address) {
1831-
addr_open[addr_cnt] = address;
1832-
addr_cnt++;
1833-
}
18341806
}
18351807
break;
18361808
case DISC|PF:
@@ -1841,33 +1813,8 @@ static void gsm_queue(struct gsm_mux *gsm)
18411813
return;
18421814
}
18431815
/* Real close complete */
1844-
if (!address) {
1845-
if (addr_cnt > 0) {
1846-
for (i = 0; i < addr_cnt; i++) {
1847-
address = addr_open[i];
1848-
dlci = gsm->dlci[address];
1849-
gsm_dlci_close(dlci);
1850-
addr_open[i] = 0;
1851-
}
1852-
}
1853-
dlci = gsm->dlci[0];
1854-
gsm_dlci_close(dlci);
1855-
addr_cnt = 0;
1856-
gsm_response(gsm, 0, UA|PF);
1857-
} else {
1858-
gsm_response(gsm, address, UA|PF);
1859-
gsm_dlci_close(dlci);
1860-
/* clear dlci address */
1861-
for (j = 0; j < addr_cnt; j++) {
1862-
address_tmp = addr_open[j];
1863-
if (address_tmp == address) {
1864-
for (k = j; k < addr_cnt; k++)
1865-
addr_open[k] = addr_open[k+1];
1866-
addr_cnt--;
1867-
break;
1868-
}
1869-
}
1870-
}
1816+
gsm_response(gsm, address, UA|PF);
1817+
gsm_dlci_close(dlci);
18711818
break;
18721819
case UA|PF:
18731820
if (cr == 0 || dlci == NULL)
@@ -2451,19 +2398,17 @@ static int gsmld_attach_gsm(struct tty_struct *tty, struct gsm_mux *gsm)
24512398
else {
24522399
/* Don't register device 0 - this is the control channel and not
24532400
a usable tty interface */
2454-
if (gsm->initiator) {
2455-
base = mux_num_to_base(gsm); /* Base for this MUX */
2456-
for (i = 1; i < NUM_DLCI; i++) {
2457-
struct device *dev;
2401+
base = mux_num_to_base(gsm); /* Base for this MUX */
2402+
for (i = 1; i < NUM_DLCI; i++) {
2403+
struct device *dev;
24582404

2459-
dev = tty_register_device(gsm_tty_driver,
2405+
dev = tty_register_device(gsm_tty_driver,
24602406
base + i, NULL);
2461-
if (IS_ERR(dev)) {
2462-
for (i--; i >= 1; i--)
2463-
tty_unregister_device(gsm_tty_driver,
2464-
base + i);
2465-
return PTR_ERR(dev);
2466-
}
2407+
if (IS_ERR(dev)) {
2408+
for (i--; i >= 1; i--)
2409+
tty_unregister_device(gsm_tty_driver,
2410+
base + i);
2411+
return PTR_ERR(dev);
24672412
}
24682413
}
24692414
}
@@ -2485,10 +2430,8 @@ static void gsmld_detach_gsm(struct tty_struct *tty, struct gsm_mux *gsm)
24852430
int i;
24862431

24872432
WARN_ON(tty != gsm->tty);
2488-
if (gsm->initiator) {
2489-
for (i = 1; i < NUM_DLCI; i++)
2490-
tty_unregister_device(gsm_tty_driver, base + i);
2491-
}
2433+
for (i = 1; i < NUM_DLCI; i++)
2434+
tty_unregister_device(gsm_tty_driver, base + i);
24922435
tty_kref_put(gsm->tty);
24932436
gsm->tty = NULL;
24942437
}

0 commit comments

Comments
 (0)