Re: [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()

[Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>]

On 06/21/2013 01:49 AM, Wei Yongjun wrote:
> On 06/21/2013 04:07 PM, Zhang Rui wrote:
>> On Tue, 2013-06-18 at 21:09 +0800, Wei Yongjun wrote:
>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>
>>> If krealloc() returns NULL, it doesn't free the original. So any code
>>> of the form 'foo = krealloc(foo, ...);' is almost certainly a bug.
>>>
>> you mean this would probably cause a memory leak, right?
Agree. But whether system will really function it can't allocate few 
bytes (=number CPU  packages). It will crash somewhere anyway.
> Yes.
>
>>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>> ---
>>>   drivers/thermal/x86_pkg_temp_thermal.c | 8 +++++---
>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/thermal/x86_pkg_temp_thermal.c b/drivers/thermal/x86_pkg_temp_thermal.c
>>> index 5de56f6..b90e84b 100644
>>> --- a/drivers/thermal/x86_pkg_temp_thermal.c
>>> +++ b/drivers/thermal/x86_pkg_temp_thermal.c
>>> @@ -394,6 +394,7 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
>>>   	char buffer[30];
>>>   	int thres_count;
>>>   	u32 eax, ebx, ecx, edx;
>>> +	u8 *temp;
>>>   
>>>   	cpuid(6, &eax, &ebx, &ecx, &edx);
>>>   	thres_count = ebx & 0x07;
>>> @@ -417,13 +418,14 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
>>>   	spin_lock(&pkg_work_lock);
>>>   	if (topology_physical_package_id(cpu) > max_phy_id)
>>>   		max_phy_id = topology_physical_package_id(cpu);
>>> -	pkg_work_scheduled = krealloc(pkg_work_scheduled,
>>> -				(max_phy_id+1) * sizeof(u8), GFP_ATOMIC);
>>> -	if (!pkg_work_scheduled) {
>>> +	temp = krealloc(pkg_work_scheduled,
>>> +			(max_phy_id+1) * sizeof(u8), GFP_ATOMIC);
>>> +	if (!temp) {
>> without this patch, this function will quite if krealloc returns NULL,
>> but with the previous pkg_work_scheduled unfreed, right?
> Yes, without patch, previous pkg_work_scheduled will unfreed if krealloc
> return NULL.
>
>> thanks,
>> rui
>>>   		spin_unlock(&pkg_work_lock);
>>>   		err = -ENOMEM;
>>>   		goto err_ret_free;
>>>   	}
>>> +	pkg_work_scheduled = temp;
>>>   	pkg_work_scheduled[topology_physical_package_id(cpu)] = 0;
>>>   	spin_unlock(&pkg_work_lock);
>>>   
>>>
>>
>>
>
>