[RFC] tty: n_gsm: fix use-after-free in gsmtty_install

[Kito Xu (veritas501)]

gsmtty_install() reads gsm_mux[] without holding gsm_mux_lock and
defers mux_get() about 40 lines later. A concurrent gsmld_close() can
free the mux through gsm_cleanup_mux() -> mux_put() -> kfree(gsm) in
that window, leading to a use-after-free when gsmtty_install() later
dereferences the stale pointer.

race condition:

         cpu 0                          |            cpu 1
    gsmtty_install()                    |   gsmld_close()
    gsm = gsm_mux[mux] // no lock      |
    // CFS preempts here                |   gsm_cleanup_mux(gsm)
                                        |   gsm->dead = true
                                        |   mux_put(gsm)
                                        |    -> kfree(gsm)
    gsm->dead           // UAF!        |
    mutex_lock(&gsm->mutex) // UAF!    |

Acquire gsm_mux_lock before reading gsm_mux[] and take the refcount
via kref_get() while still holding the lock, so the mux cannot be
freed between lookup and refcount increment.

Fixes: 86176ed90545 ("TTY: n_gsm, use tty_port_install")
Signed-off-by: Kito Xu (veritas501) <hxzene@xxxxxxxxx>
---
 drivers/tty/n_gsm.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de83b..6519f1c92fc5 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -4288,14 +4288,20 @@ static int gsmtty_install(struct tty_driver *driver, struct tty_struct *tty)
 
 	if (mux >= MAX_MUX)
 		return -ENXIO;
-	/* FIXME: we need to lock gsm_mux for lifetimes of ttys eventually */
-	if (gsm_mux[mux] == NULL)
-		return -EUNATCH;
 	if (line == 0 || line > 61)	/* 62/63 reserved */
 		return -ECHRNG;
+
+	/* Acquire gsm_mux_lock to prevent concurrent gsmld_close() from
+	 * freeing the mux between reading gsm_mux[] and taking a refcount.
+	 */
+	spin_lock(&gsm_mux_lock);
 	gsm = gsm_mux[mux];
-	if (gsm->dead)
-		return -EL2HLT;
+	if (!gsm || gsm->dead) {
+		spin_unlock(&gsm_mux_lock);
+		return gsm ? -EL2HLT : -EUNATCH;
+	}
+	kref_get(&gsm->ref);
+	spin_unlock(&gsm_mux_lock);
 	/* If DLCI 0 is not yet fully open return an error.
 	This is ok from a locking
 	perspective as we don't have to worry about this
@@ -4309,8 +4315,10 @@ static int gsmtty_install(struct tty_driver *driver, struct tty_struct *tty)
 		if (dlci0->state == DLCI_OPENING)
 			wait_event(gsm->event, dlci0->state != DLCI_OPENING);
 
-		if (dlci0->state != DLCI_OPEN)
+		if (dlci0->state != DLCI_OPEN) {
+			mux_put(gsm);
 			return -EL2NSYNC;
+		}
 
 		mutex_lock(&gsm->mutex);
 	}
@@ -4322,6 +4330,7 @@ static int gsmtty_install(struct tty_driver *driver, struct tty_struct *tty)
 	}
 	if (dlci == NULL) {
 		mutex_unlock(&gsm->mutex);
+		mux_put(gsm);
 		return -ENOMEM;
 	}
 	ret = tty_port_install(&dlci->port, driver, tty);
@@ -4329,12 +4338,12 @@ static int gsmtty_install(struct tty_driver *driver, struct tty_struct *tty)
 		if (alloc)
 			dlci_put(dlci);
 		mutex_unlock(&gsm->mutex);
+		mux_put(gsm);
 		return ret;
 	}
 
 	dlci_get(dlci);
 	dlci_get(gsm->dlci[0]);
-	mux_get(gsm);
 	tty->driver_data = dlci;
 	mutex_unlock(&gsm->mutex);
 
-- 
2.43.0