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

[Greg KH <gregkh@linuxfoundation.org>]

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