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 <[email protected]> Reviewed-by: Zhang Yanmin <[email protected]> --- 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
