linux-kernel - Re: [PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error handle case in alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Dec 2021 12:56:02 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Yang Yingliang <yangyingliang@...wei.com>
Cc:     linux-kernel@...r.kernel.org, linux-staging@...ts.linux.dev,
        gregkh@...uxfoundation.org, paskripkin@...il.com
Subject: Re: [PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error
 handle case in alloc_rtllib()

Almost perfect, but it needs one minor change.

On Wed, Dec 01, 2021 at 05:50:35PM +0800, Yang Yingliang wrote:
> Some variables are leaked in the error handling in alloc_rtllib(), free
> the variables in the error path.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@...wei.com>
> ---

Please send this as a v4 patch with a little note here:

v4: Fix crypt_info leak
v3: Fix more leaks.  Break it up into multple patches.
v2: Make rtllib_softmac_init() return error codes.

You can probably put that in the 0/3 email.


>  drivers/staging/rtl8192e/rtllib_module.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_module.c b/drivers/staging/rtl8192e/rtllib_module.c
> index 64d9feee1f39..a3c74fa25cfa 100644
> --- a/drivers/staging/rtl8192e/rtllib_module.c
> +++ b/drivers/staging/rtl8192e/rtllib_module.c
> @@ -88,7 +88,7 @@ struct net_device *alloc_rtllib(int sizeof_priv)
>  	err = rtllib_networks_allocate(ieee);
>  	if (err) {
>  		pr_err("Unable to allocate beacon storage: %d\n", err);
> -		goto failed;
> +		goto free_netdev;
>  	}
>  	rtllib_networks_initialize(ieee);
>  
> @@ -121,11 +121,13 @@ struct net_device *alloc_rtllib(int sizeof_priv)
>  	ieee->hwsec_active = 0;
>  
>  	memset(ieee->swcamtable, 0, sizeof(struct sw_cam_table) * 32);
> -	rtllib_softmac_init(ieee);
> +	err = rtllib_softmac_init(ieee);
> +	if (err)
> +		goto free_networks;

This needs to free crypt_info;  This was my mistake in the email I sent
earlier.  Sorry!

>  
>  	ieee->pHTInfo = kzalloc(sizeof(struct rt_hi_throughput), GFP_KERNEL);
>  	if (!ieee->pHTInfo)
> -		return NULL;
> +		goto free_softmac;
>  
>  	HTUpdateDefaultSetting(ieee);
>  	HTInitializeHTInfo(ieee);
> @@ -141,8 +143,14 @@ struct net_device *alloc_rtllib(int sizeof_priv)
>  
>  	return dev;
>  
> - failed:
> +free_softmac:
> +	rtllib_softmac_free(ieee);
> +	lib80211_crypt_info_free(&ieee->crypt_info);
> +free_networks:
> +	rtllib_networks_free(ieee);
> +free_netdev:
>  	free_netdev(dev);
> +
>  	return NULL;

Something like:

free_softmac:
	rtllib_softmac_free(ieee);
free_crypt_info:
	lib80211_crypt_info_free(&ieee->crypt_info);
	rtllib_networks_free(ieee);
free_netdev:
 	free_netdev(dev);

regards,
dan carpenter