[PATCH] tty/n_gsm.c: do not clear gsm_mux entry when the gsm is not closed

From: xinhui.pan
Date: Thu Jul 24 2014 - 05:19:09 EST


If the gsmtty is still used by some process, we could not just
simply clear gsm_mux[gsm->num]. Clear it when gsm is being free.
Otherwise we will hit crashes when userspace close the gsmtty.

Also add gsm_mux_get() and gsm_mux_put() to make gsm_mux[] is used safely.
We can do activation/deactivation with same gsm more than once now.
This is for fixing the FIXME.

Signed-off-by: xinhui.pan <xinhuiX.pan@xxxxxxxxx>
Reviewed-by: Zhang Yanmin <yanmin_zhang@xxxxxxxxxxxxxxx>
---
drivers/tty/n_gsm.c | 84 ++++++++++++++++++++++++++++++++++++---------------
1 file changed, 60 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 81e7ccb..290df56 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2020,6 +2020,58 @@ static void gsm_error(struct gsm_mux *gsm,
}

/**
+ * gsm_mux_get - get/fill one entry in gsm_mux
+ * @gsm: our gsm
+ *
+ * Although its name end with get, it don't inc ref-count actually.
+ * get one entry is just like fill pte, first memory access will
+ * cause page_fault, the next accesses don't. So do here.
+ */
+
+static int gsm_mux_get(struct gsm_mux *gsm)
+{
+ int i;
+
+ if (gsm->num >= MAX_MUX) /* gsm is alloc by kzalloc, just be careful */
+ return -EIO;
+ if (gsm_mux[gsm->num] == gsm) /* We have already set gsm->num */
+ return 0;
+
+ spin_lock(&gsm_mux_lock);
+ for (i = 0; i < MAX_MUX; i++) {
+ if (gsm_mux[i] == NULL) {
+ gsm->num = i;
+ gsm_mux[i] = gsm;
+ break;
+ }
+ }
+ spin_unlock(&gsm_mux_lock);
+
+ if (i == MAX_MUX)
+ return -EBUSY;
+ return 0;
+}
+
+/**
+ * gsm_mux_put - put/clear one entry in gsm_mux
+ * @gsm: our gsm
+ *
+ * Although its name end with put, it don't dec ref-count actually.
+ * put one entry is just like clear pte, So do here.
+ */
+
+static void gsm_mux_put(struct gsm_mux *gsm)
+{
+ if (gsm->num >= MAX_MUX)
+ return;
+
+ spin_lock(&gsm_mux_lock);
+ if (gsm_mux[gsm->num] == gsm)
+ gsm_mux[gsm->num] = NULL;
+ spin_unlock(&gsm_mux_lock);
+}
+
+/**
* gsm_cleanup_mux - generic GSM protocol cleanup
* @gsm: our mux
*
@@ -2037,16 +2089,6 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm)

gsm->dead = 1;

- spin_lock(&gsm_mux_lock);
- for (i = 0; i < MAX_MUX; i++) {
- if (gsm_mux[i] == gsm) {
- gsm_mux[i] = NULL;
- break;
- }
- }
- spin_unlock(&gsm_mux_lock);
- WARN_ON(i == MAX_MUX);
-
/* In theory disconnecting DLCI 0 is sufficient but for some
modems this is apparently not the case. */
if (dlci) {
@@ -2086,7 +2128,7 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm)
static int gsm_activate_mux(struct gsm_mux *gsm)
{
struct gsm_dlci *dlci;
- int i = 0;
+ int ret = 0;

init_timer(&gsm->t2_timer);
gsm->t2_timer.function = gsm_control_retransmit;
@@ -2101,17 +2143,12 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
gsm->receive = gsm1_receive;
gsm->error = gsm_error;

- spin_lock(&gsm_mux_lock);
- for (i = 0; i < MAX_MUX; i++) {
- if (gsm_mux[i] == NULL) {
- gsm->num = i;
- gsm_mux[i] = gsm;
- break;
- }
- }
- spin_unlock(&gsm_mux_lock);
- if (i == MAX_MUX)
- return -EBUSY;
+ /*
+ * call gsm_mux_get more than once is safe with same gsm
+ */
+ ret = gsm_mux_get(gsm);
+ if (ret)
+ return ret;

dlci = gsm_dlci_alloc(gsm, 0);
if (dlci == NULL)
@@ -2142,6 +2179,7 @@ static void gsm_free_mux(struct gsm_mux *gsm)
static void gsm_free_muxr(struct kref *ref)
{
struct gsm_mux *gsm = container_of(ref, struct gsm_mux, ref);
+ gsm_mux_put(gsm);
gsm_free_mux(gsm);
}

@@ -2559,8 +2597,6 @@ static int gsmld_config(struct tty_struct *tty, struct gsm_mux *gsm,
if (c->t2)
gsm->t2 = c->t2;

- /* FIXME: We need to separate activation/deactivation from adding
- and removing from the mux array */
if (need_restart)
gsm_activate_mux(gsm);
if (gsm->initiator && need_close)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/