[PATCH] tty: n_gsm: fix NULL deref of gsm->dlci[0] in control message handlers

[PATCH] tty: n_gsm: fix NULL deref of gsm->dlci[0] in control message handlers

[Weiming Shi]

gsm_control_command() and gsm_control_reply() load gsm->dlci[0] and
immediately dereference dlci->ftype without checking it for NULL.

On the receive path, gsm_queue() validates that gsm->dlci[0] is non-NULL
and DLCI_OPEN before invoking the control handler, but the value is not
held across that check: the receive worker runs from flush_to_ldisc()
without taking gsm->mutex, while a concurrent GSMIOC_SETCONF ioctl can
enter gsm_cleanup_mux(), which takes gsm->mutex, releases gsm->dlci[0]
and sets it to NULL. If the mux is torn down between gsm_queue()'s check
and the re-load inside gsm_control_command()/gsm_control_reply(), the
handler dereferences a NULL dlci.

A peer that drives DLCI 0 control frames (e.g. CMD_TEST) while the mux
owner reconfigures the line discipline can therefore crash the kernel
(line numbers from decode_stacktrace.sh against the crashing build):

 Oops: general protection fault, probably for non-canonical address
 KASAN: null-ptr-deref in range [0x0000000000000208-0x000000000000020f]
 RIP: 0010:gsm_control_reply (drivers/tty/n_gsm.c:1497)
 Call Trace:
  gsm_dlci_command (drivers/tty/n_gsm.c:2482)
  gsm_queue.part.0 (drivers/tty/n_gsm.c:2852)
  gsm0_receive (drivers/tty/n_gsm.c:2972)
  gsmld_receive_buf (drivers/tty/n_gsm.c:3629)
  tty_ldisc_receive_buf (drivers/tty/tty_buffer.c:391)
  tty_port_default_receive_buf (drivers/tty/tty_port.c:39)
  flush_to_ldisc (drivers/tty/tty_buffer.c:495)
  process_one_work
  worker_thread
  kthread

The other callers of these helpers (the keep-alive and negotiation timer
paths) already guard the gsm->dlci[0] access; only the receive path is
unguarded. The CMD_CLD handler in the same switch already checks the
loaded dlci for NULL for the very same reason. Bail out early when
gsm->dlci[0] has been cleared instead of dereferencing it.

Triggering this requires CAP_NET_ADMIN to attach the n_gsm line
discipline (gsmld_open() uses capable(), not ns_capable()), so it is a
local denial of service for a privileged mux owner racing its own
control channel; harden the handlers regardless.

Fixes: 5767712668b8 ("tty: n_gsm: cleanup gsm_control_command and gsm_control_reply")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/tty/n_gsm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 214abeb89aaa..860cfb91d510 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1457,6 +1457,9 @@ static int gsm_control_command(struct gsm_mux *gsm, int cmd, const u8 *data,
 	struct gsm_msg *msg;
 	struct gsm_dlci *dlci = gsm->dlci[0];
 
+	if (!dlci)
+		return -EINVAL;
+
 	msg = gsm_data_alloc(gsm, 0, dlen + 2, dlci->ftype);
 	if (msg == NULL)
 		return -ENOMEM;
@@ -1485,6 +1488,9 @@ static void gsm_control_reply(struct gsm_mux *gsm, int cmd, const u8 *data,
 	struct gsm_msg *msg;
 	struct gsm_dlci *dlci = gsm->dlci[0];
 
+	if (!dlci)
+		return;
+
 	msg = gsm_data_alloc(gsm, 0, dlen + 2, dlci->ftype);
 	if (msg == NULL)
 		return;
-- 
2.43.0

Re: [PATCH] tty: n_gsm: fix NULL deref of gsm->dlci[0] in control message handlers

[Greg Kroah-Hartman]

On Thu, Jun 11, 2026 at 11:32:18AM -0700, Weiming Shi wrote:
> gsm_control_command() and gsm_control_reply() load gsm->dlci[0] and
> immediately dereference dlci->ftype without checking it for NULL.
> 
> On the receive path, gsm_queue() validates that gsm->dlci[0] is non-NULL
> and DLCI_OPEN before invoking the control handler, but the value is not
> held across that check: the receive worker runs from flush_to_ldisc()
> without taking gsm->mutex, while a concurrent GSMIOC_SETCONF ioctl can
> enter gsm_cleanup_mux(), which takes gsm->mutex, releases gsm->dlci[0]
> and sets it to NULL. If the mux is torn down between gsm_queue()'s check
> and the re-load inside gsm_control_command()/gsm_control_reply(), the
> handler dereferences a NULL dlci.
> 
> A peer that drives DLCI 0 control frames (e.g. CMD_TEST) while the mux
> owner reconfigures the line discipline can therefore crash the kernel
> (line numbers from decode_stacktrace.sh against the crashing build):
> 
>  Oops: general protection fault, probably for non-canonical address
>  KASAN: null-ptr-deref in range [0x0000000000000208-0x000000000000020f]
>  RIP: 0010:gsm_control_reply (drivers/tty/n_gsm.c:1497)
>  Call Trace:
>   gsm_dlci_command (drivers/tty/n_gsm.c:2482)
>   gsm_queue.part.0 (drivers/tty/n_gsm.c:2852)
>   gsm0_receive (drivers/tty/n_gsm.c:2972)
>   gsmld_receive_buf (drivers/tty/n_gsm.c:3629)
>   tty_ldisc_receive_buf (drivers/tty/tty_buffer.c:391)
>   tty_port_default_receive_buf (drivers/tty/tty_port.c:39)
>   flush_to_ldisc (drivers/tty/tty_buffer.c:495)
>   process_one_work
>   worker_thread
>   kthread
> 
> The other callers of these helpers (the keep-alive and negotiation timer
> paths) already guard the gsm->dlci[0] access; only the receive path is
> unguarded. The CMD_CLD handler in the same switch already checks the
> loaded dlci for NULL for the very same reason. Bail out early when
> gsm->dlci[0] has been cleared instead of dereferencing it.
> 
> Triggering this requires CAP_NET_ADMIN to attach the n_gsm line
> discipline (gsmld_open() uses capable(), not ns_capable()), so it is a
> local denial of service for a privileged mux owner racing its own
> control channel; harden the handlers regardless.
> 
> Fixes: 5767712668b8 ("tty: n_gsm: cleanup gsm_control_command and gsm_control_reply")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  drivers/tty/n_gsm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 214abeb89aaa..860cfb91d510 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -1457,6 +1457,9 @@ static int gsm_control_command(struct gsm_mux *gsm, int cmd, const u8 *data,
>  	struct gsm_msg *msg;
>  	struct gsm_dlci *dlci = gsm->dlci[0];
>  
> +	if (!dlci)
> +		return -EINVAL;

What precents dlci from being NULL right after you check this?

thanks,

greg k-h