[PATCH] tty: n_gsm: fix memory leak in gsm_activate_mux

[PATCH] tty: n_gsm: fix memory leak in gsm_activate_mux

[Minu Jin]

I found a memory leak in gsm_activate_mux() from the syzbot dashboard.
I have analyzed the root cause and created a fix.
Please test the following patch.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-next

---
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de83b..de3d30eac86e 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2645,7 +2645,12 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in

 static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
 {
-       struct gsm_dlci *dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
+       struct gsm_dlci *dlci;
+
+       if (gsm->dlci[addr])
+               return gsm->dlci[addr];
+
+       dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
        if (dlci == NULL)
                return NULL;
        spin_lock_init(&dlci->lock);
@@ -3196,8 +3201,10 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
                gsm->receive = gsm1_receive;

        ret = gsm_register_devices(gsm_tty_driver, gsm->num);
-       if (ret)
+       if (ret) {
+               gsm_dlci_free(&dlci->port);
                return ret;
+       }

        gsm->has_devices = true;
        gsm->dead = false;              /* Tty opens are now permissible */

[PATCH] tty: n_gsm: fix memory leak in gsm_activate_mux

[Minu Jin]

syzbot reported a memory leak in gsm_activate_mux().
The root cause is a missing cleanup path when gsm_register_devices()
fails. In this case, the previously allocated DLCI 0
and its associated kfifo remain allocated, leading to a memory leak.

And gsm_dlci_alloc() does not check for already allocated DLCIs.
Repeated calls to gsm_activate_mux() would overwrite the existing pointer
in gsm->dlci[addr], causing the original memory to be lost.

Fix this by:
1. Adding gsm_dlci_free() in the error path of gsm_activate_mux().
2. Adding a check in gsm_dlci_alloc() to return the existing DLCI
   if it is already allocated.

Reported-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b5d1f455d385b2c7da3c
Tested-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
Fixes: 01aecd917114 ("tty: n_gsm: fix tty registration before control channel open")
Signed-off-by: Minu Jin <s9430939@naver.com>
---
 drivers/tty/n_gsm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de83b..de3d30eac86e 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2645,7 +2645,12 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
 
 static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
 {
-	struct gsm_dlci *dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
+	struct gsm_dlci *dlci;
+
+	if (gsm->dlci[addr])
+		return gsm->dlci[addr];
+
+	dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
 	if (dlci == NULL)
 		return NULL;
 	spin_lock_init(&dlci->lock);
@@ -3196,8 +3201,10 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
 		gsm->receive = gsm1_receive;
 
 	ret = gsm_register_devices(gsm_tty_driver, gsm->num);
-	if (ret)
+	if (ret) {
+		gsm_dlci_free(&dlci->port);
 		return ret;
+	}
 
 	gsm->has_devices = true;
 	gsm->dead = false;		/* Tty opens are now permissible */
-- 
2.43.0

Re: [PATCH] tty: n_gsm: fix memory leak in gsm_activate_mux

[Greg KH]

On Thu, Apr 23, 2026 at 03:33:21AM +0900, Minu Jin wrote:
> syzbot reported a memory leak in gsm_activate_mux().
> The root cause is a missing cleanup path when gsm_register_devices()
> fails. In this case, the previously allocated DLCI 0
> and its associated kfifo remain allocated, leading to a memory leak.
> 
> And gsm_dlci_alloc() does not check for already allocated DLCIs.
> Repeated calls to gsm_activate_mux() would overwrite the existing pointer
> in gsm->dlci[addr], causing the original memory to be lost.
> 
> Fix this by:
> 1. Adding gsm_dlci_free() in the error path of gsm_activate_mux().
> 2. Adding a check in gsm_dlci_alloc() to return the existing DLCI
>    if it is already allocated.
> 
> Reported-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b5d1f455d385b2c7da3c
> Tested-by: syzbot+b5d1f455d385b2c7da3c@syzkaller.appspotmail.com
> Fixes: 01aecd917114 ("tty: n_gsm: fix tty registration before control channel open")
> Signed-off-by: Minu Jin <s9430939@naver.com>
> ---
>  drivers/tty/n_gsm.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c13e050de83b..de3d30eac86e 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2645,7 +2645,12 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
>  
>  static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
>  {
> -	struct gsm_dlci *dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
> +	struct gsm_dlci *dlci;
> +
> +	if (gsm->dlci[addr])
> +		return gsm->dlci[addr];

Why would you be allocating a device twice?  Shouldn't that logic be
fixed instead?

> +
> +	dlci = kzalloc_obj(struct gsm_dlci, GFP_ATOMIC);
>  	if (dlci == NULL)
>  		return NULL;
>  	spin_lock_init(&dlci->lock);
> @@ -3196,8 +3201,10 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
>  		gsm->receive = gsm1_receive;
>  
>  	ret = gsm_register_devices(gsm_tty_driver, gsm->num);
> -	if (ret)
> +	if (ret) {
> +		gsm_dlci_free(&dlci->port);
>  		return ret;
> +	}

How was this tested?

thanks,

greg k-h