On Fri, Oct 26, 2012 at 10:11:45AM +0200, Guillaume Juan wrote: > From: Guillaume Juan <[email protected]> > > If gsm->tty happens to be NULL in gsmld_output, avoid crashing the kernel > (the crash is replaced by a warning dump).
How can ->tty be NULL here? > Prevent at earlier level such situation: > - gsmtty_hangup does no longer call gsm_dlci_begin_close when called > synchronously from gsm_cleanup_mux, because this would arm a timer that will > asynchronously lead to use gms->tty, potentially after it is nullified. > - in gsm_cleanup_mux, release DLCIs (other than DLCI#0 which is the control > one) before closing mux. Else, it can happen that the T2 timer is rearmed by > another thread scheduled between del_timer_sync and gsm_dlci release > (for example by the following call-stack: fput => tty_release => > tty_port_close_start => tty_port_lower_dtr_rts => gsm_dtr_rts => > gsmtty_modem_update) Please wrap your changelog comments at 72 columns. > > Signed-off-by: Guillaume Juan <[email protected]> > --- > drivers/tty/n_gsm.c | 35 ++++++++++++++++++++++++++--------- > 1 files changed, 26 insertions(+), 9 deletions(-) > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index 1e8e8ce..fe3c6ad 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -1510,8 +1510,8 @@ static void gsm_dlci_begin_open(struct gsm_dlci *dlci) > } > > /** > - * gsm_dlci_begin_close - start channel open procedure > - * @dlci: DLCI to open > + * gsm_dlci_begin_close - start channel close procedure > + * @dlci: DLCI to close > * > * Commence closing a DLCI from the Linux side. We issue DISC messages > * to the modem which should then reply with a UA, at which point we > @@ -2031,6 +2031,13 @@ void gsm_cleanup_mux(struct gsm_mux *gsm) > spin_unlock(&gsm_mux_lock); > WARN_ON(i == MAX_MUX); > > + /* Free up any link layer users */ > + if (dlci) > + dlci->dead = 1; > + for (i = 1; i < NUM_DLCI; i++) > + if (gsm->dlci[i]) > + gsm_dlci_release(gsm->dlci[i]); > + > /* In theory disconnecting DLCI 0 is sufficient but for some > modems this is apparently not the case. */ > if (dlci) { > @@ -2041,15 +2048,11 @@ void gsm_cleanup_mux(struct gsm_mux *gsm) > del_timer_sync(&gsm->t2_timer); > /* Now we are sure T2 has stopped */ > if (dlci) { > - dlci->dead = 1; > gsm_dlci_begin_close(dlci); > wait_event_interruptible(gsm->event, > dlci->state == DLCI_CLOSED); > + gsm_dlci_release(dlci); > } > - /* Free up any link layer users */ > - for (i = 0; i < NUM_DLCI; i++) > - if (gsm->dlci[i]) > - gsm_dlci_release(gsm->dlci[i]); > /* Now wipe the queues */ > list_for_each_entry_safe(txq, ntxq, &gsm->tx_list, list) > kfree(txq); > @@ -2192,6 +2195,10 @@ EXPORT_SYMBOL_GPL(gsm_alloc_mux); > > static int gsmld_output(struct gsm_mux *gsm, u8 *data, int len) > { > + if (gsm->tty == NULL) { > + WARN(1, "NULL gsm->tty pointer in gsmld_output\n"); > + return -EINVAL; > + } > if (tty_write_room(gsm->tty) < len) { > set_bit(TTY_DO_WRITE_WAKEUP, &gsm->tty->flags); > return -ENOSPC; > @@ -2323,7 +2330,7 @@ static void gsmld_flush_buffer(struct tty_struct *tty) > * @tty: device > * > * Called from the terminal layer when this line discipline is > - * being shut down, either because of a close or becsuse of a > + * being shut down, either because of a close or because of a > * discipline change. The function will not be called while other > * ldisc methods are in progress. > */ > @@ -2954,6 +2961,15 @@ static void gsmtty_close(struct tty_struct *tty, > struct file *filp) > gsm = dlci->gsm; > if (tty_port_close_start(&dlci->port, tty, filp) == 0) > goto out; > + /* Should not happen because the DLCI ttys are hung up (which causes > + tty_port_close_start to return 0) by gsm_dlci_release before setting > + gsm->tty to NULL */ > + if (gsm->tty == NULL) { > + WARN(1, "NULL gsm->tty pointer in gsmtty_close for %s\n", > + tty->name); > + goto out; > + } > + > gsm_dlci_begin_close(dlci); > tty_port_close_end(&dlci->port, tty); > tty_port_tty_set(&dlci->port, NULL); > @@ -2967,7 +2983,8 @@ static void gsmtty_hangup(struct tty_struct *tty) > { > struct gsm_dlci *dlci = tty->driver_data; > tty_port_hangup(&dlci->port); > - gsm_dlci_begin_close(dlci); > + if (!dlci->gsm->dead) > + gsm_dlci_begin_close(dlci); > } > > static int gsmtty_write(struct tty_struct *tty, const unsigned char *buf, > -- > 1.7.0.4 > > > > # > " Ce courriel et les documents qui lui sont joints peuvent contenir des > informations confidentielles ou ayant un caractère privé. S'ils ne vous sont > pas destinés, nous vous signalons qu'il est strictement interdit de les > divulguer, de les reproduire ou d'en utiliser de quelque manière que ce > soit le contenu. Si ce message vous a été transmis par erreur, merci d'en > informer l'expéditeur et de supprimer immédiatement de votre système > informatique ce courriel ainsi que tous les documents qui y sont attachés." > > > ****** > > " This e-mail and any attached documents may contain confidential or > proprietary information. If you are not the intended recipient, you are > notified that any dissemination, copying of this e-mail and any attachments > thereto or use of their contents by any means whatsoever is strictly > prohibited. If you have received this e-mail in error, please advise the > sender immediately and delete this e-mail and all attached documents > from your computer system." > # Because of that footer, I am not allowed to accept this patch at all. Please remove it and resend. thanks, greg k-h -- 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/
