linux-kernel - Re: [PATCH 1/2] Thermal:Fix memory leak if occur goto unregister

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141103183510.GA441@developer>
Date:	Mon, 3 Nov 2014 14:35:13 -0400
From:	Eduardo Valentin <edubezval@...il.com>
To:	"R, Durgadoss" <durgadoss.r@...el.com>
Cc:	Yao Dongdong <yaodongdong@...wei.com>,
	"Zhang, Rui" <rui.zhang@...el.com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] Thermal:Fix memory leak if occur goto unregister

Hello,

On Mon, Nov 03, 2014 at 06:17:37PM +0000, R, Durgadoss wrote:
> >-----Original Message-----
> >From: linux-pm-owner@...r.kernel.org [mailto:linux-pm-
> >owner@...r.kernel.org] On Behalf Of Eduardo Valentin
> >Sent: Monday, October 20, 2014 6:04 PM
> >To: Yao Dongdong
> >Cc: Zhang, Rui; linux-pm@...r.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@...wei.com
> >
> >Acked-by: Eduardo Valentin <edubezval@...il.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.

Thanks,


Eduardo Valentin

> >>     return ERR_PTR(result);
> >>  }
> >>  EXPORT_SYMBOL_GPL(thermal_zone_device_register);
> >> --
> >> 1.8.0.1
> >>
> >>

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)