Subject: Re: [PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error
 handle case in alloc_rtllib()
To:     Dan Carpenter <dan.carpenter@oracle.com>
CC:     <linux-kernel@vger.kernel.org>, <linux-staging@lists.linux.dev>,
        <gregkh@linuxfoundation.org>, <paskripkin@gmail.com>
References: <20211201095036.1763163-1-yangyingliang@huawei.com>
 <20211201095036.1763163-3-yangyingliang@huawei.com>
 <20211201095602.GB18178@kadam>
From:   Yang Yingliang <yangyingliang@huawei.com>
Message-ID: <31d8310f-edae-da8f-7ac4-e780db8a9973@huawei.com>
Date:   Thu, 2 Dec 2021 10:41:07 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20211201095602.GB18178@kadam>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


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