* [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()
@ 2013-06-18 13:09 Wei Yongjun
2013-06-21 8:07 ` Zhang Rui
0 siblings, 1 reply; 6+ messages in thread
From: Wei Yongjun @ 2013-06-18 13:09 UTC (permalink / raw)
To: rui.zhang, eduardo.valentin; +Cc: yongjun_wei, linux-pm
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.
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) {
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()
2013-06-18 13:09 [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add() Wei Yongjun
@ 2013-06-21 8:07 ` Zhang Rui
2013-06-21 8:49 ` Wei Yongjun
0 siblings, 1 reply; 6+ messages in thread
From: Zhang Rui @ 2013-06-21 8:07 UTC (permalink / raw)
To: Wei Yongjun; +Cc: eduardo.valentin, yongjun_wei, linux-pm, Srinivas Pandruvada
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?
> 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?
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);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()
2013-06-21 8:07 ` Zhang Rui
@ 2013-06-21 8:49 ` Wei Yongjun
2013-06-21 16:36 ` Srinivas Pandruvada
2013-07-12 0:36 ` Srinivas Pandruvada
0 siblings, 2 replies; 6+ messages in thread
From: Wei Yongjun @ 2013-06-21 8:49 UTC (permalink / raw)
To: rui.zhang; +Cc: eduardo.valentin, yongjun_wei, linux-pm, srinivas.pandruvada
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?
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);
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()
2013-06-21 8:49 ` Wei Yongjun
@ 2013-06-21 16:36 ` Srinivas Pandruvada
2013-07-12 0:36 ` Srinivas Pandruvada
1 sibling, 0 replies; 6+ messages in thread
From: Srinivas Pandruvada @ 2013-06-21 16:36 UTC (permalink / raw)
To: Wei Yongjun; +Cc: rui.zhang, eduardo.valentin, yongjun_wei, linux-pm
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);
>>>
>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()
2013-06-21 8:49 ` Wei Yongjun
2013-06-21 16:36 ` Srinivas Pandruvada
@ 2013-07-12 0:36 ` Srinivas Pandruvada
2013-07-15 8:19 ` Zhang Rui
1 sibling, 1 reply; 6+ messages in thread
From: Srinivas Pandruvada @ 2013-07-12 0:36 UTC (permalink / raw)
To: Wei Yongjun; +Cc: rui.zhang, eduardo.valentin, yongjun_wei, linux-pm
Tested this patch and it works fine.
Thanks,
Srinivas
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?
> 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);
>>>
>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add()
2013-07-12 0:36 ` Srinivas Pandruvada
@ 2013-07-15 8:19 ` Zhang Rui
0 siblings, 0 replies; 6+ messages in thread
From: Zhang Rui @ 2013-07-15 8:19 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Wei Yongjun, eduardo.valentin, yongjun_wei, linux-pm
On Thu, 2013-07-11 at 17:36 -0700, Srinivas Pandruvada wrote:
> Tested this patch and it works fine.
>
> Thanks,
> Srinivas
>
> 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?
> > Yes.
> >
> >>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
applied to thermal-next.
thanks,
rui
> >>> ---
> >>> 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);
> >>>
> >>>
> >>
> >>
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-15 8:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-18 13:09 [PATCH -next] Thermal: x86_pkg_temp: fix krealloc() misuse in in pkg_temp_thermal_device_add() Wei Yongjun
2013-06-21 8:07 ` Zhang Rui
2013-06-21 8:49 ` Wei Yongjun
2013-06-21 16:36 ` Srinivas Pandruvada
2013-07-12 0:36 ` Srinivas Pandruvada
2013-07-15 8:19 ` Zhang Rui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).