[PATCH] tty: n_gsm: pin DLCI during configuration ioctls

[Qing Ming <a0yami@mailbox.org>]

The N_GSM line discipline keeps per-channel state in the mux table
gsm->dlci[].  GSMIOC_GETCONF_DLCI and GSMIOC_SETCONF_DLCI fetch a raw
pointer from that table and then read or update the DLCI
configuration, allocating the entry on the spot when the slot is
empty.

The table lookup is done without holding gsm->mutex and without
taking a reference on the DLCI.  A concurrent GSMIOC_SETCONF that
changes the multiplexer configuration calls gsm_cleanup_mux(), which
releases every table entry under gsm->mutex; the final tty_port put
frees the DLCI.  The first ioctl then keeps using the freed object:
the GET path reads it in gsm_dlci_copy_config_values() and the SET
path writes it in gsm_dlci_config().

The issue was found by static analysis of the returned-pointer
lifetime.  A reproducer on a PTY-backed passive mux racing 16
concurrent GSMIOC_GETCONF_DLCI readers against repeated mux restarts
via GSMIOC_SETCONF, with no fault injection or artificial delays,
produced the same KASAN report in 2 of 2 unpatched boots: the first
report landed within half a minute of starting the race, and each
120-second race window averaged 2-3 reports per restart (486 reports
across 172 restarts, then 357 across 162; the copy helper is inlined,
so KASAN names the caller):

  BUG: KASAN: slab-use-after-free in gsmld_ioctl+0x156b/0x17e0
  Read of size 4 at addr ffff88800b84d008 by task n_gsm_dlci_conf/226

  Call Trace:
   gsmld_ioctl
   tty_ioctl
   __x64_sys_ioctl

  Allocated by task 218:
   gsm_dlci_alloc
   gsmld_ioctl

  Freed by task 235:
   kfree
   gsm_cleanup_mux
   gsmld_ioctl

The reported address is 8 bytes inside the freed 2048-byte object
(cache kmalloc-2k), i.e. the read of dlci->addr.

A control run with this patch applied completed 19291547 GET
iterations and 226 restarts with no KASAN report.

Serialize the lookup/allocation against mux cleanup with gsm->mutex
and acquire a tty_port reference before dropping it.  A concurrent
gsm_cleanup_mux() then either completes before the lookup, in which
case the slot is NULL and a fresh DLCI is allocated, or runs after
the reference is taken, in which case its put is not final and the
object survives until the ioctl releases it.

Fixes: afe3154ba87e ("tty: n_gsm: add ioctl for DLC config via ldisc handle")
Cc: stable@vger.kernel.org
Signed-off-by: Qing Ming <a0yami@mailbox.org>
Assisted-by: Claude:kimi-k3
---
 drivers/tty/n_gsm.c | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de83b..605549d742e6 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2711,6 +2711,22 @@ static inline void dlci_put(struct gsm_dlci *dlci)
 	tty_port_put(&dlci->port);
 }
 
+static struct gsm_dlci *gsm_dlci_get_or_alloc(struct gsm_mux *gsm,
+					      unsigned int addr)
+{
+	struct gsm_dlci *dlci;
+
+	mutex_lock(&gsm->mutex);
+	dlci = gsm->dlci[addr];
+	if (!dlci)
+		dlci = gsm_dlci_alloc(gsm, addr);
+	if (dlci)
+		dlci_get(dlci);
+	mutex_unlock(&gsm->mutex);
+
+	return dlci;
+}
+
 static void gsm_destroy_network(struct gsm_dlci *dlci);
 
 /**
@@ -3862,29 +3878,29 @@ static int gsmld_ioctl(struct tty_struct *tty, unsigned int cmd,
 		if (dc.channel == 0 || dc.channel >= NUM_DLCI)
 			return -EINVAL;
 		addr = array_index_nospec(dc.channel, NUM_DLCI);
-		dlci = gsm->dlci[addr];
-		if (!dlci) {
-			dlci = gsm_dlci_alloc(gsm, addr);
-			if (!dlci)
-				return -ENOMEM;
-		}
+		dlci = gsm_dlci_get_or_alloc(gsm, addr);
+		if (!dlci)
+			return -ENOMEM;
 		gsm_dlci_copy_config_values(dlci, &dc);
+		dlci_put(dlci);
 		if (copy_to_user((void __user *)arg, &dc, sizeof(dc)))
 			return -EFAULT;
 		return 0;
-	case GSMIOC_SETCONF_DLCI:
+	case GSMIOC_SETCONF_DLCI: {
+		int ret;
+
 		if (copy_from_user(&dc, (void __user *)arg, sizeof(dc)))
 			return -EFAULT;
 		if (dc.channel == 0 || dc.channel >= NUM_DLCI)
 			return -EINVAL;
 		addr = array_index_nospec(dc.channel, NUM_DLCI);
-		dlci = gsm->dlci[addr];
-		if (!dlci) {
-			dlci = gsm_dlci_alloc(gsm, addr);
-			if (!dlci)
-				return -ENOMEM;
-		}
-		return gsm_dlci_config(dlci, &dc, 0);
+		dlci = gsm_dlci_get_or_alloc(gsm, addr);
+		if (!dlci)
+			return -ENOMEM;
+		ret = gsm_dlci_config(dlci, &dc, 0);
+		dlci_put(dlci);
+		return ret;
+	}
 	default:
 		return n_tty_ioctl_helper(tty, cmd, arg);
 	}

base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.53.0