public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC] tty: n_gsm: fix use-after-free in gsmtty_install
@ 2026-04-15  2:48 Kito Xu (veritas501)
  2026-04-15  3:17 ` [PATCH] " Kito Xu (veritas501)
  0 siblings, 1 reply; 2+ messages in thread
From: Kito Xu (veritas501) @ 2026-04-15  2:48 UTC (permalink / raw)
  To: gregkh, jirislaby; +Cc: linux-serial, linux-kernel, 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@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH] tty: n_gsm: fix use-after-free in gsmtty_install
  2026-04-15  2:48 [RFC] tty: n_gsm: fix use-after-free in gsmtty_install Kito Xu (veritas501)
@ 2026-04-15  3:17 ` Kito Xu (veritas501)
  0 siblings, 0 replies; 2+ messages in thread
From: Kito Xu (veritas501) @ 2026-04-15  3:17 UTC (permalink / raw)
  To: gregkh, jirislaby; +Cc: linux-serial, linux-kernel, 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!    |

KASAN report:

 BUG: KASAN: slab-use-after-free in gsmtty_install+0x6cf/0x830
 Read of size 1 at addr ffff88800fd440ac by task poc/170

 CPU: 0 UID: 0 PID: 170 Comm: poc Not tainted 7.0.0-rc7-next-20260410+ #20
 Call Trace:
  <TASK>
  dump_stack_lvl+0x64/0x80
  print_report+0xd0/0x5e0
  kasan_report+0xce/0x100
  gsmtty_install+0x6cf/0x830
  tty_init_dev.part.0+0x92/0x4a0
  tty_open+0x8ab/0x1050
  chrdev_open+0x1ec/0x5e0
  do_dentry_open+0x419/0x1260
  vfs_open+0x79/0x350
  path_openat+0x212c/0x3a70
  do_file_open+0x1d2/0x400
  do_sys_openat2+0xdc/0x170
  __x64_sys_openat+0x122/0x1e0
  do_syscall_64+0x64/0x680
  entry_SYSCALL_64_after_hwframe+0x76/0x7e
  </TASK>

 Allocated by task 169 on cpu 0 at 3.824684s:
 Freed by task 169 on cpu 0 at 3.892012s:

 The buggy address belongs to the object at ffff88800fd44000
  which belongs to the cache kmalloc-1k of size 1024
 The buggy address is located 172 bytes inside of
  freed 1024-byte region [ffff88800fd44000, ffff88800fd44400)

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@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-15  3:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15  2:48 [RFC] tty: n_gsm: fix use-after-free in gsmtty_install Kito Xu (veritas501)
2026-04-15  3:17 ` [PATCH] " Kito Xu (veritas501)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox