From: Yao Dongdong <yaodongdong@huawei.com>
To: Eduardo Valentin <edubezval@gmail.com>,
"R, Durgadoss" <durgadoss.r@intel.com>
Cc: "Zhang, Rui" <rui.zhang@intel.com>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] Thermal:Fix memory leak if occur goto unregister
Date: Tue, 4 Nov 2014 10:01:19 +0800 [thread overview]
Message-ID: <5458336F.6090600@huawei.com> (raw)
In-Reply-To: <20141103183510.GA441@developer>
On 2014/11/4 2:35, Eduardo Valentin wrote:
> Hello,
>
> On Mon, Nov 03, 2014 at 06:17:37PM +0000, R, Durgadoss wrote:
>>> -----Original Message-----
>>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>>> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
>>> Sent: Monday, October 20, 2014 6:04 PM
>>> To: Yao Dongdong
>>> Cc: Zhang, Rui; linux-pm@vger.kernel.org; LKML
>>> Subject: Re: [PATCH 1/2] Thermal:Fix memory leak if occur goto unregister
>>>
>>> Hello,
>>>
>>> On Mon, Oct 20, 2014 at 04:27:36PM +0800, Yao Dongdong wrote:
>>>> Signed-off-by:yaodongdong@huawei.com
>>> Acked-by: Eduardo Valentin <edubezval@gmail.com>
>>>
>>> Rui, would you take care of this?
>>>
>> If I remember it right, this 'tz' is freed in the thermal_release()
>> function, during device_unregister().
>>
>> It is similar in all other functions in thermal_core.c
>>
>> So, Yao, Did you really test this patch ?
>> And did not see any crashes ?
>>
> Yao, reading the patch change carefully, now I see that Durga is correct
> here.
>
>> Thanks,
>> Durga
>>
>>>> ---
>>>> drivers/thermal/thermal_core.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>> index 71b0ec0..5b7d466 100644
>>>> --- a/drivers/thermal/thermal_core.c
>>>> +++ b/drivers/thermal/thermal_core.c
>>>> @@ -1574,6 +1574,7 @@ struct thermal_zone_device
>>> *thermal_zone_device_register(const char *type,
>>>> unregister:
>>>> release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
>>>> device_unregister(&tz->device);
>>>> + kfree(tz);
> The device unregister is called above your kfree call, which will cause
> the thermal_release to be called. Did you test the code for double kfree
> calls? Your patch probably inserts a memory corruption.
>
> I will revert your patch from my tree until you provide an answer about
> your testing.
Yes, i have checked it and you are right, the patch will double kfree tz.
Thansks Durga for correcting me.
> Thanks,
>
>
> Eduardo Valentin
>
>>>> return ERR_PTR(result);
>>>> }
>>>> EXPORT_SYMBOL_GPL(thermal_zone_device_register);
>>>> --
>>>> 1.8.0.1
>>>>
>>>>
prev parent reply other threads:[~2014-11-04 2:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-20 8:27 [PATCH 1/2] Thermal:Fix memory leak if occur goto unregister Yao Dongdong
2014-10-20 8:27 ` Yao Dongdong
2014-10-20 12:33 ` Eduardo Valentin
2014-11-03 18:17 ` R, Durgadoss
2014-11-03 18:35 ` Eduardo Valentin
2014-11-04 2:01 ` Yao Dongdong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5458336F.6090600@huawei.com \
--to=yaodongdong@huawei.com \
--cc=durgadoss.r@intel.com \
--cc=edubezval@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rui.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.