[PATCH -next 0/3] some fix and cleanup for rtl8192e

[PATCH -next 0/3] some fix and cleanup for rtl8192e

[Yang Yingliang]

patch #1, #2:
  fix error handle case in alloc_rtllib()

patch #3:
  remove unnecessary assignment

Yang Yingliang (3):
  staging: rtl8192e: return error code from rtllib_softmac_init()
  staging: rtl8192e: rtllib_module: fix error handle case in
    alloc_rtllib()
  staging: rtl8192e: rtllib_module: remove unnecessary assignment

 drivers/staging/rtl8192e/rtllib.h         |  2 +-
 drivers/staging/rtl8192e/rtllib_module.c  | 17 ++++++++++++-----
 drivers/staging/rtl8192e/rtllib_softmac.c |  6 ++++--
 3 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH -next 1/3] staging: rtl8192e: return error code from rtllib_softmac_init()

[Yang Yingliang]

If it fails to allocate 'dot11d_info', rtllib_softmac_init()
should return error code. And remove unneccessary error message.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/staging/rtl8192e/rtllib.h         | 2 +-
 drivers/staging/rtl8192e/rtllib_softmac.c | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index c6f8b772335c..c985e4ebc545 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -1980,7 +1980,7 @@ void SendDisassociation(struct rtllib_device *ieee, bool deauth, u16 asRsn);
 void rtllib_softmac_xmit(struct rtllib_txb *txb, struct rtllib_device *ieee);
 
 void rtllib_start_ibss(struct rtllib_device *ieee);
-void rtllib_softmac_init(struct rtllib_device *ieee);
+int rtllib_softmac_init(struct rtllib_device *ieee);
 void rtllib_softmac_free(struct rtllib_device *ieee);
 void rtllib_disassociate(struct rtllib_device *ieee);
 void rtllib_stop_scan(struct rtllib_device *ieee);
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index aabbea48223d..4b6c2295a3cf 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -2952,7 +2952,7 @@ void rtllib_start_protocol(struct rtllib_device *ieee)
 	}
 }
 
-void rtllib_softmac_init(struct rtllib_device *ieee)
+int rtllib_softmac_init(struct rtllib_device *ieee)
 {
 	int i;
 
@@ -2963,7 +2963,8 @@ void rtllib_softmac_init(struct rtllib_device *ieee)
 		ieee->seq_ctrl[i] = 0;
 	ieee->dot11d_info = kzalloc(sizeof(struct rt_dot11d_info), GFP_ATOMIC);
 	if (!ieee->dot11d_info)
-		netdev_err(ieee->dev, "Can't alloc memory for DOT11D\n");
+		return -ENOMEM;
+
 	ieee->LinkDetectInfo.SlotIndex = 0;
 	ieee->LinkDetectInfo.SlotNum = 2;
 	ieee->LinkDetectInfo.NumRecvBcnInPeriod = 0;
@@ -3029,6 +3030,7 @@ void rtllib_softmac_init(struct rtllib_device *ieee)
 
 	tasklet_setup(&ieee->ps_task, rtllib_sta_ps);
 
+	return 0;
 }
 
 void rtllib_softmac_free(struct rtllib_device *ieee)
-- 
2.25.1

[PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error handle case in alloc_rtllib()

[Yang Yingliang]

Some variables are leaked in the error handling in alloc_rtllib(), free
the variables in the error path.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 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;
 
 	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;
 }
 EXPORT_SYMBOL(alloc_rtllib);
-- 
2.25.1

[PATCH -next 3/3] staging: rtl8192e: rtllib_module: remove unnecessary assignment

[Yang Yingliang]

Remove the null pointer assignment after freeing 'ieee->pHTInfo'.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/staging/rtl8192e/rtllib_module.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtllib_module.c b/drivers/staging/rtl8192e/rtllib_module.c
index a3c74fa25cfa..52d1c3f5f3aa 100644
--- a/drivers/staging/rtl8192e/rtllib_module.c
+++ b/drivers/staging/rtl8192e/rtllib_module.c
@@ -161,7 +161,6 @@ void free_rtllib(struct net_device *dev)
 				      netdev_priv_rsl(dev);
 
 	kfree(ieee->pHTInfo);
-	ieee->pHTInfo = NULL;
 	rtllib_softmac_free(ieee);
 
 	lib80211_crypt_info_free(&ieee->crypt_info);
-- 
2.25.1

Re: [PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error handle case in alloc_rtllib()

[Dan Carpenter]

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@huawei.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

Re: [PATCH -next 0/3] some fix and cleanup for rtl8192e

[Pavel Skripkin]

On 12/1/21 12:50, Yang Yingliang wrote:
> patch #1, #2:
>    fix error handle case in alloc_rtllib()
> 
> patch #3:
>    remove unnecessary assignment
> 
> Yang Yingliang (3):
>    staging: rtl8192e: return error code from rtllib_softmac_init()
>    staging: rtl8192e: rtllib_module: fix error handle case in
>      alloc_rtllib()
>    staging: rtl8192e: rtllib_module: remove unnecessary assignment
> 
>   drivers/staging/rtl8192e/rtllib.h         |  2 +-
>   drivers/staging/rtl8192e/rtllib_module.c  | 17 ++++++++++++-----
>   drivers/staging/rtl8192e/rtllib_softmac.c |  6 ++++--
>   3 files changed, 17 insertions(+), 8 deletions(-)
> 

Agree with Dan, almost perfect one. Since you are going to send v4, 
please add following tag to 1/3 and 2/3 patches because they should be 
backported to stable kernels

Fixes: 94a799425eee ("From: wlanfae <wlanfae@realtek.com>")

( Im not kidding, it's real subject line of the patch :) )





With regards,
Pavel Skripkin

Re: [PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error handle case in alloc_rtllib()

[Yang Yingliang]

On 2021/12/1 17:56, Dan Carpenter wrote:
> 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@huawei.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);
Yes, you are right, I should find this mistake before I send the patch.
I will send a v4 patch set with this fix.

Thanks,
Yang
>
> regards,
> dan carpenter
>
> .