The receive worker (flush_to_ldisc -> gsmld_receive_buf -> gsm0_receive/
gsm1_receive -> gsm_queue) reads gsm->dlci[address] and dispatches the
frame via dlci->data() without holding gsm->mutex. The control handlers
reached through dlci->data() then re-read gsm->dlci[]: gsm_control_reply()
re-reads gsm->dlci[0], while gsm_control_modem() (MSC), gsm_control_rls()
(RLS) and gsm_control_negotiation() (PN) re-read gsm->dlci[addr] for the
DLCI named in the command - a different channel from the one the frame
was addressed to.
Concurrently GSMIOC_SETCONF -> gsm_config() -> gsm_cleanup_mux() takes
gsm->mutex and releases every DLCI via gsm_dlci_release() -> dlci_put().
When the last reference is dropped the destructor gsm_dlci_free() clears
gsm->dlci[addr] and frees the object. If the worker dereferences one of
those DLCIs while it is being freed, it touches freed memory.
A peer that drives DLCI 0 control frames (e.g. CMD_TEST) while the mux
owner reconfigures the line discipline with GSMIOC_SETCONF can therefore
trigger a use-after-free:
BUG: KASAN: slab-use-after-free in gsm_control_reply.isra.0
Read of size 8 at addr ffff888029ae9000 by task kworker/u16:2/46
Workqueue: events_unbound flush_to_ldisc
Call Trace:
gsm_control_reply.isra.0 (drivers/tty/n_gsm.c:1494)
gsm_dlci_command (drivers/tty/n_gsm.c:2477)
gsmld_receive_buf (drivers/tty/n_gsm.c:3616)
tty_ldisc_receive_buf (drivers/tty/tty_buffer.c:398)
tty_port_default_receive_buf (drivers/tty/tty_port.c:37)
flush_to_ldisc (drivers/tty/tty_buffer.c:502)
process_one_work
worker_thread
kthread
Freed by task 5110:
kfree
gsm_cleanup_mux (drivers/tty/n_gsm.c:3161)
gsmld_ioctl (drivers/tty/n_gsm.c:3415)
tty_ioctl
Pin each DLCI across the dereference with its existing tty_port reference.
gsm_dlci_open_get() looks gsm->dlci[addr] up under gsm->mutex and, if
present, takes a dlci_get() reference before dropping the mutex; the
caller releases it with gsm_dlci_unget() once it is done. While the
reference is held the kref cannot reach zero, so gsm_dlci_free() cannot
run and the object stays live. gsm_queue() pins the addressed DLCI for
the UI/UIH dispatch, and gsm_control_modem(), gsm_control_rls() and
gsm_control_negotiation() each pin the DLCI they operate on; the addr
range check stays at the call site so a malformed frame cannot index
gsm->dlci[] out of bounds.
The reference is taken only under the mutex, around the lookup; the mutex
is released before dlci->data() and before the data-path work
(gsm_process_modem(), tty_flip_buffer_push(), gsm_data_queue(), ...), so
the receive/transmit path is not serialised by gsm->mutex and its timing
is unaffected.
Attaching the n_gsm line discipline requires CAP_NET_ADMIN (gsmld_open()
uses capable(), not ns_capable()), so this is a local denial of service
for a privileged mux owner whose control channel is driven by an
untrusted peer on the serial link while it reconfigures; harden the
receive path regardless.
Fixes: 6ab8fba7fcb0 ("tty: n_gsm: Added refcount usage to gsm_mux and gsm_dlci
structs")
Cc: [email protected]
Reported-by: Xiang Mei <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <[email protected]>
---
drivers/tty/n_gsm.c | 73 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 67 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de..771b49000 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -453,6 +453,8 @@ static const u8 gsm_fcs8[256] = {
#define GOOD_FCS 0xCF
static void gsm_dlci_close(struct gsm_dlci *dlci);
+static struct gsm_dlci *gsm_dlci_open_get(struct gsm_mux *gsm, unsigned int
addr);
+static void gsm_dlci_unget(struct gsm_dlci *dlci);
static int gsmld_output(struct gsm_mux *gsm, u8 *data, int len);
static int gsm_modem_update(struct gsm_dlci *dlci, u8 brk);
static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len,
@@ -1695,9 +1697,8 @@ static void gsm_control_modem(struct gsm_mux *gsm, const
u8 *data, int clen)
addr >>= 1;
/* Closed port, or invalid ? */
- if (addr == 0 || addr >= NUM_DLCI || gsm->dlci[addr] == NULL)
+ if (addr == 0 || addr >= NUM_DLCI)
return;
- dlci = gsm->dlci[addr];
/* Must be at least one byte following the EA */
if ((cl - len) < 1)
@@ -1711,12 +1712,18 @@ static void gsm_control_modem(struct gsm_mux *gsm,
const u8 *data, int clen)
if (len < 1)
return;
+ /* Hold the DLCI until we are done; see gsm_dlci_open_get(). */
+ dlci = gsm_dlci_open_get(gsm, addr);
+ if (dlci == NULL)
+ return;
+
tty = tty_port_tty_get(&dlci->port);
gsm_process_modem(tty, dlci, modem, cl);
if (tty) {
tty_wakeup(tty);
tty_kref_put(tty);
}
+ gsm_dlci_unget(dlci);
gsm_control_reply(gsm, CMD_MSC, data, clen);
}
@@ -1746,15 +1753,22 @@ static void gsm_control_negotiation(struct gsm_mux
*gsm, unsigned int cr,
/* Invalid DLCI? */
params = (struct gsm_dlci_param_bits *)data;
addr = FIELD_GET(PN_D_FIELD_DLCI, params->d_bits);
- if (addr == 0 || addr >= NUM_DLCI || !gsm->dlci[addr]) {
+ if (addr == 0 || addr >= NUM_DLCI) {
+ gsm->open_error++;
+ return;
+ }
+
+ /* Hold the DLCI until we are done; see gsm_dlci_open_get(). */
+ dlci = gsm_dlci_open_get(gsm, addr);
+ if (dlci == NULL) {
gsm->open_error++;
return;
}
- dlci = gsm->dlci[addr];
/* Too late for parameter negotiation? */
if ((!cr && dlci->state == DLCI_OPENING) || dlci->state == DLCI_OPEN) {
gsm->open_error++;
+ gsm_dlci_unget(dlci);
return;
}
@@ -1765,6 +1779,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm,
unsigned int cr,
pr_info("%s PN failed\n", __func__);
gsm->open_error++;
gsm_dlci_close(dlci);
+ gsm_dlci_unget(dlci);
return;
}
@@ -1785,6 +1800,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm,
unsigned int cr,
pr_info("%s PN in invalid state\n", __func__);
gsm->open_error++;
}
+ gsm_dlci_unget(dlci);
}
/**
@@ -1800,6 +1816,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm,
unsigned int cr,
static void gsm_control_rls(struct gsm_mux *gsm, const u8 *data, int clen)
{
+ struct gsm_dlci *dlci;
struct tty_port *port;
unsigned int addr = 0;
u8 bits;
@@ -1817,14 +1834,18 @@ static void gsm_control_rls(struct gsm_mux *gsm, const
u8 *data, int clen)
return;
addr >>= 1;
/* Closed port, or invalid ? */
- if (addr == 0 || addr >= NUM_DLCI || gsm->dlci[addr] == NULL)
+ if (addr == 0 || addr >= NUM_DLCI)
return;
/* No error ? */
bits = *dp;
if ((bits & 1) == 0)
return;
- port = &gsm->dlci[addr]->port;
+ /* Hold the DLCI until we are done; see gsm_dlci_open_get(). */
+ dlci = gsm_dlci_open_get(gsm, addr);
+ if (dlci == NULL)
+ return;
+ port = &dlci->port;
if (bits & 2)
tty_insert_flip_char(port, 0, TTY_OVERRUN);
@@ -1835,6 +1856,7 @@ static void gsm_control_rls(struct gsm_mux *gsm, const u8
*data, int clen)
tty_flip_buffer_push(port);
+ gsm_dlci_unget(dlci);
gsm_control_reply(gsm, CMD_RLS, data, clen);
}
@@ -2711,6 +2733,35 @@ static inline void dlci_put(struct gsm_dlci *dlci)
tty_port_put(&dlci->port);
}
+/**
+ * gsm_dlci_open_get - look up a DLCI and take a reference
+ * @gsm: GSM mux
+ * @addr: DLCI address
+ *
+ * Look up gsm->dlci[addr] under gsm->mutex and take a reference. Returns
+ * NULL if not present. Release with gsm_dlci_unget().
+ */
+static struct gsm_dlci *gsm_dlci_open_get(struct gsm_mux *gsm, unsigned int
addr)
+{
+ struct gsm_dlci *dlci;
+
+ mutex_lock(&gsm->mutex);
+ dlci = gsm->dlci[addr];
+ if (dlci != NULL)
+ dlci_get(dlci);
+ mutex_unlock(&gsm->mutex);
+ return dlci;
+}
+
+/**
+ * gsm_dlci_unget - drop a reference from
gsm_dlci_open_get()
+ * @dlci: DLCI to release
+ */
+static void gsm_dlci_unget(struct gsm_dlci *dlci)
+{
+ dlci_put(dlci);
+}
+
static void gsm_destroy_network(struct gsm_dlci *dlci);
/**
@@ -2839,11 +2890,21 @@ static void gsm_queue(struct gsm_mux *gsm)
case UI|PF:
case UIH:
case UIH|PF:
+ /*
+ * Pin the DLCI so gsm_cleanup_mux() cannot free it during
+ * dispatch. The mutex is dropped before dlci->data().
+ */
+ mutex_lock(&gsm->mutex);
+ dlci = gsm->dlci[address];
if (dlci == NULL || dlci->state != DLCI_OPEN) {
+ mutex_unlock(&gsm->mutex);
gsm_response(gsm, address, DM|PF);
return;
}
+ dlci_get(dlci);
+ mutex_unlock(&gsm->mutex);
dlci->data(dlci, gsm->buf, gsm->len);
+ dlci_put(dlci);
break;
default:
goto invalid;
--
2.43.0
