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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Jun 12, 2026 at 10:19:18PM +0800, Weiming Shi wrote:
> On Fri Jun 12, 2026 at 2:50 AM CST, Greg Kroah-Hartman wrote:
> > 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
> 
> Hi greg,
> 
> I'm sorry for taking so long to respond.
> 
> After a closer look I think your review is correct.
> 
> The real problem is that the receive path touches gsm->dlci[] with no
> lock. The teardown side holds gsm->mutex while it releases and frees the
> dlci, but the receive worker does not: gsm_queue() loads
> dlci = gsm->dlci[address] while it is still valid and passes it down
> through dlci->data() to gsm_control_command()/gsm_control_reply(), which
> also re-read gsm->dlci[0] and dereference dlci->ftype.
> 
> Meanwhile GSMIOC_SETCONF -> gsm_cleanup_mux() takes gsm->mutex, closes
> DLCI0 and drops its reference via gsm_dlci_release(); the final
> tty_port_put() runs the gsm_dlci_free() destructor, which clears the slot
> and frees the object:
> 
> ```
>   dlci->gsm->dlci[dlci->addr] = NULL;
>   kfree(dlci);
> ```
> If that happens while the worker is still in the dispatch above, it ends
> up dereferencing the freed dlci. I can reproduce this as a use-after-free:
> 
> ```
> [  997.227486][   T46] BUG: KASAN: slab-use-after-free in gsm_control_reply.isra.0 (drivers/tty/n_gsm.c:1162 drivers/tty/n_gsm.c:1494)
> [  997.229052][   T46] Read of size 8 at addr ffff888029ae9000 by task kworker/u16:2/46
> [  997.230517][   T46]
> [  997.230952][   T46] CPU: 1 UID: 0 PID: 46 Comm: kworker/u16:2 Not tainted 7.1.0-rc7 #1 PREEMPT(full)
> [  997.230958][   T46] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-4
> [  997.230961][   T46] Workqueue: events_unbound flush_to_ldisc
> [  997.230969][   T46] Call Trace:
> [  997.230972][   T46]  <TASK>
> [  997.230974][   T46]  dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
> [  997.230990][   T46]  print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
> [  997.231008][   T46]  kasan_report (mm/kasan/report.c:595)
> [  997.231016][   T46]  gsm_control_reply.isra.0 (drivers/tty/n_gsm.c:1162 drivers/tty/n_gsm.c:1494)
> [  997.231020][   T46]  gsm_dlci_command (drivers/tty/n_gsm.c:1873 drivers/tty/n_gsm.c:2477)
> [  997.231036][   T46]  gsmld_receive_buf (drivers/tty/n_gsm.c:3616)
> [  997.231044][   T46]  tty_ldisc_receive_buf (drivers/tty/tty_buffer.c:398)
> [  997.231052][   T46]  tty_port_default_receive_buf (drivers/tty/tty_port.c:37)
> [  997.231056][   T46]  flush_to_ldisc (drivers/tty/tty_buffer.c:452 drivers/tty/tty_buffer.c:502)
> [  997.231066][   T46]  process_one_work (kernel/workqueue.c:3314)
> [  997.231082][   T46]  worker_thread (kernel/workqueue.c:3397 kernel/workqueue.c:3478)
> [  997.231091][   T46]  kthread (kernel/kthread.c:436)
> [  997.231103][   T46]  ret_from_fork (arch/x86/kernel/process.c:158)
> [  997.231120][   T46]  ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
> [  997.231128][   T46]  </TASK>
> [  997.231130][   T46]
> [  997.267905][   T46] Allocated by task 5110:
> [  997.268716][   T46]  kasan_save_stack (mm/kasan/common.c:57)
> [  997.269595][   T46]  kasan_save_track (mm/kasan/common.c:78)
> [  997.270483][   T46]  __kasan_kmalloc (mm/kasan/common.c:398 mm/kasan/common.c:415)
> [  997.271353][   T46]  gsm_dlci_alloc (./include/linux/slab.h:950 ./include/linux/slab.h:1188 drivers/tty/n_gsm.c:2648)
> [  997.272203][   T46]  gsm_activate_mux (drivers/tty/n_gsm.c:3189)
> [  997.273109][   T46]  gsmld_ioctl (drivers/tty/n_gsm.c:3443 drivers/tty/n_gsm.c:3846)
> [  997.273981][   T46]  tty_ioctl (drivers/tty/tty_io.c:2801)
> [  997.274789][   T46]  __x64_sys_ioctl (fs/ioctl.c:51 fs/ioctl.c:597 fs/ioctl.c:583 fs/ioctl.c:583)
> [  997.275682][   T46]  do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
> [  997.276544][   T46]  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
> [  997.277658][   T46]
> [  997.278108][   T46] Freed by task 5110:
> [  997.278865][   T46]  kasan_save_stack (mm/kasan/common.c:57)
> [  997.279740][   T46]  kasan_save_track (mm/kasan/common.c:78)
> [  997.280615][   T46]  kasan_save_free_info (mm/kasan/generic.c:584)
> [  997.281554][   T46]  __kasan_slab_free (mm/kasan/common.c:253 mm/kasan/common.c:285)
> [  997.282435][   T46]  kfree (./include/linux/kasan.h:235 mm/slub.c:2689 mm/slub.c:6251 mm/slub.c:6566)
> [  997.283159][   T46]  gsm_cleanup_mux (drivers/tty/n_gsm.c:2711 drivers/tty/n_gsm.c:2744 drivers/tty/n_gsm.c:3161)
> [  997.284050][   T46]  gsmld_ioctl (drivers/tty/n_gsm.c:3415 drivers/tty/n_gsm.c:3846)
> [  997.284928][   T46]  tty_ioctl (drivers/tty/tty_io.c:2801)
> [  997.285746][   T46]  __x64_sys_ioctl (fs/ioctl.c:51 fs/ioctl.c:597 fs/ioctl.c:583 fs/ioctl.c:583)
> [  997.286653][   T46]  do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
> [  997.287526][   T46]  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
> [  997.288639][   T46]
> 
> ```
> 
> The NULL deref I reported  is the same unguarded access on the same
> path, just hitting the window after the slot is already cleared. Either
> way a NULL check in the handlers can't fix it, since in the UAF case dlci
> isn't NULL.
> 
> I think the fix should serialize the receive side against
> gsm_cleanup_mux() instead of checking in the handlers. Two ways I can see:
> 
>   1. take gsm->mutex around the dlci lookup and dispatch in gsm_queue(), or
>   2. pin the dlci across the dispatch using its existing tty_port ref
>     (dlci_get/dlci_put), so gsm_dlci_free() can't run while it's in use.
> 
> Do you have a preference, or is there a pattern in n_gsm you'd rather I
> use? I'll respin v2 once I know which way to go.

The bigger issue here is that almost no one has this hardware.  And
those that do, don't care about these types of issues as they do not
have untrusted data or untrusted users, so be careful when changing
things that you aren't able to test.

I think that option 2 would probably be best, as that should not affect
any fast code paths, right?

> And I'll send the reproducer and the config to trigger it in a separate mail.

Can you turn that into a real test to be added to the tree in the
correct location?  I think we need to start adding these so that we get
a base regression test for people to be able to run as all the LLM tools
seem to love the broken code in this file :)

thanks,

greg k-h