[PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[Dmitry Antipov]

Initially reported by syzbot at as memory leak, actual issue is an attempt
to configure DLCI 0 (via 'ioctl(..., GSMIOC_SETCONF_EXT, ...)') more than
once without previous cleanup (likely requested with GSM_FL_RESTART). To
prevent such a scenario, including racy attempts to do it from multiple
threads, adjust 'gsm_activate_mux()' to grab GSM mux's mutex and throw
-EBUSY if DLCI 0 was configured already.

Reported-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b5d1f455d385b2c7da3c
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/tty/n_gsm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de83b..ce447477ccf3 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -3186,6 +3186,11 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
 	struct gsm_dlci *dlci;
 	int ret;
 
+	guard(mutex)(&gsm->mutex);
+
+	if (unlikely(gsm->dlci[0]))
+		return -EBUSY;
+
 	dlci = gsm_dlci_alloc(gsm, 0);
 	if (dlci == NULL)
 		return -ENOMEM;
-- 
2.55.0

[PATCH 2/2] tty: n_gsm: issue mutex_destroy() from gsm_dlci_free()

[Dmitry Antipov]

Although a no-op unless CONFIG_DEBUG_MUTEXES is enabled, add
missing call to 'mutex_destroy()' in 'gsm_dlci_free()'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/tty/n_gsm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index ce447477ccf3..548470796374 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2698,6 +2698,7 @@ static void gsm_dlci_free(struct tty_port *port)
 	kfifo_free(&dlci->fifo);
 	while ((dlci->skb = skb_dequeue(&dlci->skb_list)))
 		dev_kfree_skb(dlci->skb);
+	mutex_destroy(&dlci->mutex);
 	kfree(dlci);
 }
 
-- 
2.55.0

Re: [PATCH 2/2] tty: n_gsm: issue mutex_destroy() from gsm_dlci_free()

[Jiri Slaby]

On 06. 07. 26, 16:20, Dmitry Antipov wrote:
> Although a no-op unless CONFIG_DEBUG_MUTEXES is enabled, add
> missing call to 'mutex_destroy()' in 'gsm_dlci_free()'.

You somehow omitted the "why" part.

thanks,
-- 
js
suse labs

Re: [PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[Greg Kroah-Hartman]

On Mon, Jul 06, 2026 at 05:20:15PM +0300, Dmitry Antipov wrote:
> Initially reported by syzbot at as memory leak, actual issue is an attempt
> to configure DLCI 0 (via 'ioctl(..., GSMIOC_SETCONF_EXT, ...)') more than
> once without previous cleanup (likely requested with GSM_FL_RESTART). To
> prevent such a scenario, including racy attempts to do it from multiple
> threads, adjust 'gsm_activate_mux()' to grab GSM mux's mutex and throw
> -EBUSY if DLCI 0 was configured already.
> 
> Reported-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com

We've had loads of syzbot "issues" reported in this code, and unless it
actually shows up in a real device or operation, I'd just leave it alone
please as there are lots of subtle issues involved in it.

Have you tested this in real hardware?

> Closes: https://syzkaller.appspot.com/bug?extid=b5d1f455d385b2c7da3c
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/tty/n_gsm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c13e050de83b..ce447477ccf3 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -3186,6 +3186,11 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
>  	struct gsm_dlci *dlci;
>  	int ret;
>  
> +	guard(mutex)(&gsm->mutex);
> +
> +	if (unlikely(gsm->dlci[0]))
> +		return -EBUSY;

Why unlikely()?  Can you measure it with/without that?  If not, it
should never be used.

thanks,

greg k-h

Re: [PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[Dmitry Antipov]

On 7/6/26 5:35 PM, Greg Kroah-Hartman wrote:

> We've had loads of syzbot "issues" reported in this code, and unless it
> actually shows up in a real device or operation, I'd just leave it alone
> please as there are lots of subtle issues involved in it.
> 
> Have you tested this in real hardware?

No. But I don't see any practical reasons doing ioctl(..., GSMIOC_SETCONF_EXT, ...)
more than once (without GSM_FL_RESTART) without doing anything else. Worse, an
attempt to do it simultaneously from multiple threads is obviously racy.

> Why unlikely()?  Can you measure it with/without that?  If not, it
> should never be used.

See above. IIUC "real" userspace program which talks to the device is unlikely
to bomb the device driver with the weird sequences of ioctl()s like syzkaller
usually does.

Dmitry

Re: [PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[Greg Kroah-Hartman]

On Mon, Jul 06, 2026 at 05:49:38PM +0300, Dmitry Antipov wrote:
> On 7/6/26 5:35 PM, Greg Kroah-Hartman wrote:
> 
> > We've had loads of syzbot "issues" reported in this code, and unless it
> > actually shows up in a real device or operation, I'd just leave it alone
> > please as there are lots of subtle issues involved in it.
> > 
> > Have you tested this in real hardware?
> 
> No. But I don't see any practical reasons doing ioctl(..., GSMIOC_SETCONF_EXT, ...)
> more than once (without GSM_FL_RESTART) without doing anything else. Worse, an
> attempt to do it simultaneously from multiple threads is obviously racy.

That's why userspace does not do this from multiple threads :)

> > Why unlikely()?  Can you measure it with/without that?  If not, it
> > should never be used.
> 
> See above. IIUC "real" userspace program which talks to the device is unlikely
> to bomb the device driver with the weird sequences of ioctl()s like syzkaller
> usually does.

Again, if you can not measure the difference with/without unlikely()
never use it as the compiler and CPU will always do the right thing
instead.

thanks,

greg k-h

Re: [PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[Dmitry Antipov]

On 7/6/26 6:18 PM, Greg Kroah-Hartman wrote:

> That's why userspace does not do this from multiple threads :)

What about malicious userspace like https://github.com/fff-vr/n_gsm_exploit?
IIUC completely unlocked 'gsmld_ioctl()' makes such a things relatively simple.

Dmitry

Re: [PATCH 1/2] tty: n_gsm: avoid reactivation without previous cleanup

[Greg Kroah-Hartman]

On Tue, Jul 07, 2026 at 08:47:46AM +0300, Dmitry Antipov wrote:
> On 7/6/26 6:18 PM, Greg Kroah-Hartman wrote:
> 
> > That's why userspace does not do this from multiple threads :)
> 
> What about malicious userspace like https://github.com/fff-vr/n_gsm_exploit?
> IIUC completely unlocked 'gsmld_ioctl()' makes such a things relatively simple.

The gsm code should never be loaded on systems that do not have the
hardware for it, which is why it requires root access to do so.

Systems that do have this hardware, are properly secured to not allow
untrusted userspace programs on it.

This has been discussed numerous times in the past on this list, nothing
new here.

thanks,

greg k-h