* Re: mmotm 2008-12-17-16-41 uploaded [not found] <200812180057.mBI0vpPt013574@imap1.linux-foundation.org> @ 2008-12-18 4:10 ` KAMEZAWA Hiroyuki 2008-12-18 22:23 ` Greg KH 0 siblings, 1 reply; 4+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-12-18 4:10 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, gregkh On Wed, 17 Dec 2008 16:41:54 -0800 akpm@linux-foundation.org wrote: > The mm-of-the-moment snapshot 2008-12-17-16-41 has been uploaded to > > http://userweb.kernel.org/~akpm/mmotm/ > > and will soon be available at > > git://git.zen-sources.org/zen/mmotm.git > This was necessary. I'm not sure my patch description is corrcect... but this helps me ;) This change kfree(dev->p) comes from linux-next. == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> dev->release() may free struct dev itself. To free dev->p in safe way, remember dev->p. Signed-off-by:KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- drivers/base/core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) Index: mmotm-2.6.28-Dec17/drivers/base/core.c =================================================================== --- mmotm-2.6.28-Dec17.orig/drivers/base/core.c +++ mmotm-2.6.28-Dec17/drivers/base/core.c @@ -109,6 +109,7 @@ static struct sysfs_ops dev_sysfs_ops = static void device_release(struct kobject *kobj) { struct device *dev = to_dev(kobj); + struct device_private *p = dev->p; if (dev->release) dev->release(dev); @@ -116,11 +117,13 @@ static void device_release(struct kobjec dev->type->release(dev); else if (dev->class && dev->class->dev_release) dev->class->dev_release(dev); - else + else { WARN(1, KERN_ERR "Device '%s' does not have a release() " "function, it is broken and must be fixed.\n", dev_name(dev)); - kfree(dev->p); + return; + } + kfree(p); } static struct kobj_type device_ktype = { ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: mmotm 2008-12-17-16-41 uploaded 2008-12-18 4:10 ` mmotm 2008-12-17-16-41 uploaded KAMEZAWA Hiroyuki @ 2008-12-18 22:23 ` Greg KH 2008-12-19 0:11 ` KAMEZAWA Hiroyuki 0 siblings, 1 reply; 4+ messages in thread From: Greg KH @ 2008-12-18 22:23 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: linux-kernel, akpm, gregkh On Thu, Dec 18, 2008 at 01:10:15PM +0900, KAMEZAWA Hiroyuki wrote: > On Wed, 17 Dec 2008 16:41:54 -0800 > akpm@linux-foundation.org wrote: > > > The mm-of-the-moment snapshot 2008-12-17-16-41 has been uploaded to > > > > http://userweb.kernel.org/~akpm/mmotm/ > > > > and will soon be available at > > > > git://git.zen-sources.org/zen/mmotm.git > > > > This was necessary. I'm not sure my patch description is corrcect... > but this helps me ;) > This change kfree(dev->p) comes from linux-next. > > == > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > dev->release() may free struct dev itself. To free dev->p > in safe way, remember dev->p. > > Signed-off-by:KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > --- > drivers/base/core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > Index: mmotm-2.6.28-Dec17/drivers/base/core.c > =================================================================== > --- mmotm-2.6.28-Dec17.orig/drivers/base/core.c > +++ mmotm-2.6.28-Dec17/drivers/base/core.c > @@ -109,6 +109,7 @@ static struct sysfs_ops dev_sysfs_ops = > static void device_release(struct kobject *kobj) > { > struct device *dev = to_dev(kobj); > + struct device_private *p = dev->p; > > if (dev->release) > dev->release(dev); > @@ -116,11 +117,13 @@ static void device_release(struct kobjec > dev->type->release(dev); > else if (dev->class && dev->class->dev_release) > dev->class->dev_release(dev); > - else > + else { > WARN(1, KERN_ERR "Device '%s' does not have a release() " > "function, it is broken and must be fixed.\n", > dev_name(dev)); > - kfree(dev->p); > + return; > + } > + kfree(p); You have a memory leak for when there is no release function for a device, don't return if that happens. I've now fixed this up in my tree, thanks for pointing it out. greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: mmotm 2008-12-17-16-41 uploaded 2008-12-18 22:23 ` Greg KH @ 2008-12-19 0:11 ` KAMEZAWA Hiroyuki 2008-12-19 0:32 ` Greg KH 0 siblings, 1 reply; 4+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-12-19 0:11 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, akpm, gregkh On Thu, 18 Dec 2008 14:23:29 -0800 Greg KH <greg@kroah.com> wrote: > On Thu, Dec 18, 2008 at 01:10:15PM +0900, KAMEZAWA Hiroyuki wrote: > > On Wed, 17 Dec 2008 16:41:54 -0800 > > akpm@linux-foundation.org wrote: > > > > > The mm-of-the-moment snapshot 2008-12-17-16-41 has been uploaded to > > > > > > http://userweb.kernel.org/~akpm/mmotm/ > > > > > > and will soon be available at > > > > > > git://git.zen-sources.org/zen/mmotm.git > > > > > > > This was necessary. I'm not sure my patch description is corrcect... > > but this helps me ;) > > This change kfree(dev->p) comes from linux-next. > > > > == > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > > dev->release() may free struct dev itself. To free dev->p > > in safe way, remember dev->p. > > > > Signed-off-by:KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > --- > > drivers/base/core.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > Index: mmotm-2.6.28-Dec17/drivers/base/core.c > > =================================================================== > > --- mmotm-2.6.28-Dec17.orig/drivers/base/core.c > > +++ mmotm-2.6.28-Dec17/drivers/base/core.c > > @@ -109,6 +109,7 @@ static struct sysfs_ops dev_sysfs_ops = > > static void device_release(struct kobject *kobj) > > { > > struct device *dev = to_dev(kobj); > > + struct device_private *p = dev->p; > > > > if (dev->release) > > dev->release(dev); > > @@ -116,11 +117,13 @@ static void device_release(struct kobjec > > dev->type->release(dev); > > else if (dev->class && dev->class->dev_release) > > dev->class->dev_release(dev); > > - else > > + else { > > WARN(1, KERN_ERR "Device '%s' does not have a release() " > > "function, it is broken and must be fixed.\n", > > dev_name(dev)); > > - kfree(dev->p); > > + return; > > + } > > + kfree(p); > > You have a memory leak for when there is no release function for a > device, don't return if that happens. > I thought "not released" dev may touch dev->p later.. > I've now fixed this up in my tree, thanks for pointing it out. > ok, thanks. -Kame > greg k-h > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: mmotm 2008-12-17-16-41 uploaded 2008-12-19 0:11 ` KAMEZAWA Hiroyuki @ 2008-12-19 0:32 ` Greg KH 0 siblings, 0 replies; 4+ messages in thread From: Greg KH @ 2008-12-19 0:32 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: Greg KH, linux-kernel, akpm On Fri, Dec 19, 2008 at 09:11:14AM +0900, KAMEZAWA Hiroyuki wrote: > On Thu, 18 Dec 2008 14:23:29 -0800 > Greg KH <greg@kroah.com> wrote: > > > On Thu, Dec 18, 2008 at 01:10:15PM +0900, KAMEZAWA Hiroyuki wrote: > > > On Wed, 17 Dec 2008 16:41:54 -0800 > > > akpm@linux-foundation.org wrote: > > > > > > > The mm-of-the-moment snapshot 2008-12-17-16-41 has been uploaded to > > > > > > > > http://userweb.kernel.org/~akpm/mmotm/ > > > > > > > > and will soon be available at > > > > > > > > git://git.zen-sources.org/zen/mmotm.git > > > > > > > > > > This was necessary. I'm not sure my patch description is corrcect... > > > but this helps me ;) > > > This change kfree(dev->p) comes from linux-next. > > > > > > == > > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > > > > dev->release() may free struct dev itself. To free dev->p > > > in safe way, remember dev->p. > > > > > > Signed-off-by:KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > --- > > > drivers/base/core.c | 7 +++++-- > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > Index: mmotm-2.6.28-Dec17/drivers/base/core.c > > > =================================================================== > > > --- mmotm-2.6.28-Dec17.orig/drivers/base/core.c > > > +++ mmotm-2.6.28-Dec17/drivers/base/core.c > > > @@ -109,6 +109,7 @@ static struct sysfs_ops dev_sysfs_ops = > > > static void device_release(struct kobject *kobj) > > > { > > > struct device *dev = to_dev(kobj); > > > + struct device_private *p = dev->p; > > > > > > if (dev->release) > > > dev->release(dev); > > > @@ -116,11 +117,13 @@ static void device_release(struct kobjec > > > dev->type->release(dev); > > > else if (dev->class && dev->class->dev_release) > > > dev->class->dev_release(dev); > > > - else > > > + else { > > > WARN(1, KERN_ERR "Device '%s' does not have a release() " > > > "function, it is broken and must be fixed.\n", > > > dev_name(dev)); > > > - kfree(dev->p); > > > + return; > > > + } > > > + kfree(p); > > > > You have a memory leak for when there is no release function for a > > device, don't return if that happens. > > > I thought "not released" dev may touch dev->p later.. Nope, it's a bug in the caller code. They better not be touching dev->p later, or bad things would be happening to their code :) thanks, greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-12-19 0:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200812180057.mBI0vpPt013574@imap1.linux-foundation.org>
2008-12-18 4:10 ` mmotm 2008-12-17-16-41 uploaded KAMEZAWA Hiroyuki
2008-12-18 22:23 ` Greg KH
2008-12-19 0:11 ` KAMEZAWA Hiroyuki
2008-12-19 0:32 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox