tty: n_gsm: fix decoupled mux resource

The active mux instances are managed in the gsm_mux array and via mux_get()
and mux_put() functions separately. This gives a very loose coupling
between the actual instance and the gsm_mux array which manages it. It also
results in unnecessary lockings which makes it prone to failures. And it
creates a race condition if more than the maximum number of mux instances
are requested while the user changes the parameters of an active instance.
The user may loose ownership of the current mux instance in this case.
Fix this by moving the gsm_mux array handling to the mux allocation and
deallocation functions.

Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
Link: https://lore.kernel.org/r/20220414094225.4527-3-daniel.starke@siemens.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index daaffcf..f546dfe 100644 (file)
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2136,18 +2136,6 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc)
        /* Finish outstanding timers, making sure they are done */
        del_timer_sync(&gsm->t2_timer);
 
-       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);
-       /* open failed before registering => nothing to do */
-       if (i == MAX_MUX)
-               return;
-
        /* Free up any link layer users */
        for (i = 0; i < NUM_DLCI; i++)
                if (gsm->dlci[i])
@@ -2171,7 +2159,6 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc)
 static int gsm_activate_mux(struct gsm_mux *gsm)
 {
        struct gsm_dlci *dlci;
-       int i = 0;
 
        timer_setup(&gsm->t2_timer, gsm_control_retransmit, 0);
        init_waitqueue_head(&gsm->event);
@@ -2183,18 +2170,6 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
        else
                gsm->receive = gsm1_receive;
 
-       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;
-
        dlci = gsm_dlci_alloc(gsm, 0);
        if (dlci == NULL)
                return -ENOMEM;
@@ -2210,6 +2185,15 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
  */
 static void gsm_free_mux(struct gsm_mux *gsm)
 {
+       int i;
+
+       for (i = 0; i < MAX_MUX; i++) {
+               if (gsm == gsm_mux[i]) {
+                       gsm_mux[i] = NULL;
+                       break;
+               }
+       }
+       mutex_destroy(&gsm->mutex);
        kfree(gsm->txframe);
        kfree(gsm->buf);
        kfree(gsm);
@@ -2229,12 +2213,20 @@ static void gsm_free_muxr(struct kref *ref)
 
 static inline void mux_get(struct gsm_mux *gsm)
 {
+       unsigned long flags;
+
+       spin_lock_irqsave(&gsm_mux_lock, flags);
        kref_get(&gsm->ref);
+       spin_unlock_irqrestore(&gsm_mux_lock, flags);
 }
 
 static inline void mux_put(struct gsm_mux *gsm)
 {
+       unsigned long flags;
+
+       spin_lock_irqsave(&gsm_mux_lock, flags);
        kref_put(&gsm->ref, gsm_free_muxr);
+       spin_unlock_irqrestore(&gsm_mux_lock, flags);
 }
 
 static inline unsigned int mux_num_to_base(struct gsm_mux *gsm)
@@ -2255,6 +2247,7 @@ static inline unsigned int mux_line_to_num(unsigned int line)
 
 static struct gsm_mux *gsm_alloc_mux(void)
 {
+       int i;
        struct gsm_mux *gsm = kzalloc(sizeof(struct gsm_mux), GFP_KERNEL);
        if (gsm == NULL)
                return NULL;
@@ -2284,6 +2277,26 @@ static struct gsm_mux *gsm_alloc_mux(void)
        gsm->mtu = 64;
        gsm->dead = true;       /* Avoid early tty opens */
 
+       /* Store the instance to the mux array or abort if no space is
+        * available.
+        */
+       spin_lock(&gsm_mux_lock);
+       for (i = 0; i < MAX_MUX; i++) {
+               if (!gsm_mux[i]) {
+                       gsm_mux[i] = gsm;
+                       gsm->num = i;
+                       break;
+               }
+       }
+       spin_unlock(&gsm_mux_lock);
+       if (i == MAX_MUX) {
+               mutex_destroy(&gsm->mutex);
+               kfree(gsm->txframe);
+               kfree(gsm->buf);
+               kfree(gsm);
+               return NULL;
+       }
+
        return gsm;
 }