Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Mon, Mar 10, 2008 at 09:35:56PM +0530, Balaji Rao wrote:
> On Monday 10 March 2008 09:22:00 pm Greg KH wrote:
> > On Sun, Mar 09, 2008 at 12:51:15PM +0530, Balaji Rao wrote:
> > > On Sunday 09 March 2008 12:33:07 pm Greg KH wrote:
> > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > This patch does not fix it all! The problem is in fact more
> > > > > > > involved. I also get these BUG reports when I
> reload
> > > > > kvm-intel.
> > > > > > >
> > > > > > > BUG kmalloc-8: Object already free
> > > > > > > [ 74.696570]
> > > > > > > -
> > > > > > > [ 74.696596]
> > > > > > > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587
> > > > > > > cpu=2 pid=1439
> > > > > > > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32
> > > > > > > age=559 cpu=3 pid=1439
> > > > > > > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8
> > > > > > > flags=0x1c3
> > > > > > > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> > > > > > >
> > > > > > > This happens because, sysdev_class_register assigns a name to the
> > > > > > > kobject, and kfrees the old name if any. The poisoned 'name'
> > > > > > > object
> > > > > > > persists in case of statically allocated kobjects and as its
> > > > > > > passed to
> > > > > > > kfree again when re registered, we get the above warning.
> > > > > > >
> > > > > > > So, AFAICS the best way to solve this is by fixing the kobject
> > > > > > > users
> > > > > > > (kvm, oprofile etc.) to use dynamic kobjects instead of static
> > > > > > > ones or
> > > > > > > memset the kobject to zero before passing it to sysdev_register.
> > > > > >
> > > > > > I like the memset idea, how about this patch instead?
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > > >
> > > > > > --- a/drivers/base/sys.c
> > > > > > +++ b/drivers/base/sys.c
> > > > > > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
> > > > > > pr_debug("Registering sysdev class '%s'\n",
> > > > > > kobject_name(&cls->kset.kobj));
> > > > > > INIT_LIST_HEAD(&cls->drivers);
> > > > > > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > > > > > cls->kset.kobj.parent = &system_kset->kobj;
> > > > > > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > > > > > cls->kset.kobj.kset = system_kset;
> > > > > >
> > > > >
> > > > > This should work.But I am afraid if we zeroed it, the kfree poison
> > > > > test wouldn't catch even the genuine cases. Isn't it ? CMIIW.
> > > >
> > > > What "genuine cases"? kobjects should always be initialized to zero
> > > > before they are registered.
> > > >
> > > The cases in which a freed kobject is passed to the register function ?
> > > Probably they will be caught when do a memset..
> > > > > A better fix according to me would be to zero the kobject in the
> > > > > places where we know it is being re-registered (kvm, oprofile) etc.
> > > > > This should do for now. But we should fix the sys_device users later,
> > > > > for the next cycle.
> > > >
> > > > Are you sure you know all of the sysdev_class objects that can be
> > > > re-registered?
> > > >
> > > Hmm.. Right. No, I only know that KVM and Oprofile use it.
> > > > Can you test this patch out?
> > > >
> > > Yes the idea works. One more memset is needed in sysdev_register. Here's
> > > the final patch.
> > >
> > > diff --git a/drivers/base/sys.c b/drivers/base/sys.c
> > > index 2f79c55..7c839d9 100644
> > > --- a/drivers/base/sys.c
> > > +++ b/drivers/base/sys.c
> > > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
> > > pr_debug("Registering sysdev class '%s'\n",
> > > kobject_name(&cls->kset.kobj));
> > > INIT_LIST_HEAD(&cls->drivers);
> > > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > > cls->kset.kobj.parent = &system_kset->kobj;
> > > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > > cls->kset.kobj.kset = system_kset;
> > > @@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
> > >
> > > pr_debug("Registering sys device '%s'\n",
> > > kobject_name(&sysdev->kobj));
> > >
> > > + memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
> > > /* Make sure the kset is set */
> > > sysdev->kobj.kset = &cls->kset;
> >
> > Yes, you're right.
> >
> > This also means I can drop the kobject_cleanup() patch from you as well,
> > as that should no longer be needed, right?
> Yes, That's no longer needed.
>
> I am resending the patch, as the previous one had some whitespace issues.
I think that gmail did that, my original version was fine :)
thanks,
greg k-h
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012000
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sun, Mar 09, 2008 at 11:49:26AM +0100, Mikael Pettersson wrote:
> Balaji Rao writes:
> > Yes the idea works. One more memset is needed in sysdev_register. Here's
> the final patch.
> >
> > diff --git a/drivers/base/sys.c b/drivers/base/sys.c
> > index 2f79c55..7c839d9 100644
> > --- a/drivers/base/sys.c
> > +++ b/drivers/base/sys.c
> > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
> > pr_debug("Registering sysdev class '%s'\n",
> > kobject_name(&cls->kset.kobj));
> > INIT_LIST_HEAD(&cls->drivers);
> > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > cls->kset.kobj.parent = &system_kset->kobj;
> > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > cls->kset.kobj.kset = system_kset;
> > @@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
> >
> > pr_debug("Registering sys device '%s'\n",
> kobject_name(&sysdev->kobj));
> >
> > + memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
> > /* Make sure the kset is set */
> > sysdev->kobj.kset = &cls->kset;
> >
>
> Thanks, 2.6.25-rc4 + these two memset()s is finally stable for
> me with no warnings, BUG()s, or panics.
>
> (However, the patch is whitespace damaged with initial tabs
> converted to spaces.)
>
> If you want to pass this on to Linus, you can add a
>
> Tested-by: Mikael Pettersson <[EMAIL PROTECTED]>
Thanks for testing.
greg k-h
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Monday 10 March 2008 09:22:00 pm Greg KH wrote:
> On Sun, Mar 09, 2008 at 12:51:15PM +0530, Balaji Rao wrote:
> > On Sunday 09 March 2008 12:33:07 pm Greg KH wrote:
> >
> > > > > > Hi,
> > > > > >
> > > > > > This patch does not fix it all! The problem is in fact more
> > > > > > involved. I also get these BUG reports when I
reload
> > > > kvm-intel.
> > > > > >
> > > > > > BUG kmalloc-8: Object already free
> > > > > > [ 74.696570]
> > > > > > -
> > > > > > [ 74.696596]
> > > > > > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587
> > > > > > cpu=2 pid=1439
> > > > > > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32
> > > > > > age=559 cpu=3 pid=1439
> > > > > > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8
> > > > > > flags=0x1c3
> > > > > > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> > > > > >
> > > > > > This happens because, sysdev_class_register assigns a name to the
> > > > > > kobject, and kfrees the old name if any. The poisoned 'name' object
> > > > > > persists in case of statically allocated kobjects and as its passed
> > > > > > to
> > > > > > kfree again when re registered, we get the above warning.
> > > > > >
> > > > > > So, AFAICS the best way to solve this is by fixing the kobject users
> > > > > > (kvm, oprofile etc.) to use dynamic kobjects instead of static ones
> > > > > > or
> > > > > > memset the kobject to zero before passing it to sysdev_register.
> > > > >
> > > > > I like the memset idea, how about this patch instead?
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > > >
> > > > > --- a/drivers/base/sys.c
> > > > > +++ b/drivers/base/sys.c
> > > > > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
> > > > > pr_debug("Registering sysdev class '%s'\n",
> > > > >kobject_name(&cls->kset.kobj));
> > > > > INIT_LIST_HEAD(&cls->drivers);
> > > > > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > > > > cls->kset.kobj.parent = &system_kset->kobj;
> > > > > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > > > > cls->kset.kobj.kset = system_kset;
> > > > >
> > > >
> > > > This should work.But I am afraid if we zeroed it, the kfree poison
> > > > test wouldn't catch even the genuine cases. Isn't it ? CMIIW.
> > >
> > > What "genuine cases"? kobjects should always be initialized to zero
> > > before they are registered.
> > >
> > The cases in which a freed kobject is passed to the register function ?
> > Probably they will be caught when do a memset..
> > > > A better fix according to me would be to zero the kobject in the
> > > > places where we know it is being re-registered (kvm, oprofile) etc.
> > > > This should do for now. But we should fix the sys_device users later,
> > > > for the next cycle.
> > >
> > > Are you sure you know all of the sysdev_class objects that can be
> > > re-registered?
> > >
> > Hmm.. Right. No, I only know that KVM and Oprofile use it.
> > > Can you test this patch out?
> > >
> > Yes the idea works. One more memset is needed in sysdev_register. Here's
> > the final patch.
> >
> > diff --git a/drivers/base/sys.c b/drivers/base/sys.c
> > index 2f79c55..7c839d9 100644
> > --- a/drivers/base/sys.c
> > +++ b/drivers/base/sys.c
> > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
> > pr_debug("Registering sysdev class '%s'\n",
> > kobject_name(&cls->kset.kobj));
> > INIT_LIST_HEAD(&cls->drivers);
> > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > cls->kset.kobj.parent = &system_kset->kobj;
> > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > cls->kset.kobj.kset = system_kset;
> > @@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
> >
> > pr_debug("Registering sys device '%s'\n",
> > kobject_name(&sysdev->kobj));
> >
> > + memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
> > /* Make sure the kset is set */
> > sysdev->kobj.kset = &cls->kset;
>
> Yes, you're right.
>
> This also means I can drop the kobject_cleanup() patch from you as well,
> as that should no longer be needed, right?
Yes, That's no longer needed.
I am resending the patch, as the previous one had some whitespace issues.
Signed-off-by: Balaji Rao <[EMAIL PROTECTED]>
Tested-by: Mikael Pettersson <[EMAIL PROTECTED]>
diff --git a/drivers/base/sys.c b/drivers/base/sys.c
index 2f79c55..7c839d9 100644
--- a/drivers/base/sys.c
+++ b/drivers/base/sys.c
@@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
pr_debug("Registering sysdev class '%s'\n",
kobject_name(&cls->kset.kobj));
INIT_LIST_HEAD(&cls->drivers);
+ memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
cls->kset.kobj.parent = &system_kset->kobj;
cls->k
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sun, Mar 09, 2008 at 12:51:15PM +0530, Balaji Rao wrote:
> On Sunday 09 March 2008 12:33:07 pm Greg KH wrote:
>
> > > > > Hi,
> > > > >
> > > > > This patch does not fix it all! The problem is in fact more involved.
> > > > > I also get these BUG reports when I reload
> > > kvm-intel.
> > > > >
> > > > > BUG kmalloc-8: Object already free
> > > > > [ 74.696570]
> > > > > -
> > > > > [ 74.696596]
> > > > > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587
> > > > > cpu=2 pid=1439
> > > > > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32
> > > > > age=559 cpu=3 pid=1439
> > > > > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8
> > > > > flags=0x1c3
> > > > > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> > > > >
> > > > > This happens because, sysdev_class_register assigns a name to the
> > > > > kobject, and kfrees the old name if any. The poisoned 'name' object
> > > > > persists in case of statically allocated kobjects and as its passed to
> > > > > kfree again when re registered, we get the above warning.
> > > > >
> > > > > So, AFAICS the best way to solve this is by fixing the kobject users
> > > > > (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> > > > > memset the kobject to zero before passing it to sysdev_register.
> > > >
> > > > I like the memset idea, how about this patch instead?
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > > >
> > > > --- a/drivers/base/sys.c
> > > > +++ b/drivers/base/sys.c
> > > > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
> > > > pr_debug("Registering sysdev class '%s'\n",
> > > > kobject_name(&cls->kset.kobj));
> > > > INIT_LIST_HEAD(&cls->drivers);
> > > > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > > > cls->kset.kobj.parent = &system_kset->kobj;
> > > > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > > > cls->kset.kobj.kset = system_kset;
> > > >
> > >
> > > This should work.But I am afraid if we zeroed it, the kfree poison
> > > test wouldn't catch even the genuine cases. Isn't it ? CMIIW.
> >
> > What "genuine cases"? kobjects should always be initialized to zero
> > before they are registered.
> >
> The cases in which a freed kobject is passed to the register function ?
> Probably they will be caught when do a memset..
> > > A better fix according to me would be to zero the kobject in the
> > > places where we know it is being re-registered (kvm, oprofile) etc.
> > > This should do for now. But we should fix the sys_device users later,
> > > for the next cycle.
> >
> > Are you sure you know all of the sysdev_class objects that can be
> > re-registered?
> >
> Hmm.. Right. No, I only know that KVM and Oprofile use it.
> > Can you test this patch out?
> >
> Yes the idea works. One more memset is needed in sysdev_register. Here's the
> final patch.
>
> diff --git a/drivers/base/sys.c b/drivers/base/sys.c
> index 2f79c55..7c839d9 100644
> --- a/drivers/base/sys.c
> +++ b/drivers/base/sys.c
> @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
> pr_debug("Registering sysdev class '%s'\n",
> kobject_name(&cls->kset.kobj));
> INIT_LIST_HEAD(&cls->drivers);
> + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> cls->kset.kobj.parent = &system_kset->kobj;
> cls->kset.kobj.ktype = &ktype_sysdev_class;
> cls->kset.kobj.kset = system_kset;
> @@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
>
> pr_debug("Registering sys device '%s'\n",
> kobject_name(&sysdev->kobj));
>
> + memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
> /* Make sure the kset is set */
> sysdev->kobj.kset = &cls->kset;
Yes, you're right.
This also means I can drop the kobject_cleanup() patch from you as well,
as that should no longer be needed, right?
thanks,
greg k-h
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
Balaji Rao writes:
> Yes the idea works. One more memset is needed in sysdev_register. Here's the
> final patch.
>
> diff --git a/drivers/base/sys.c b/drivers/base/sys.c
> index 2f79c55..7c839d9 100644
> --- a/drivers/base/sys.c
> +++ b/drivers/base/sys.c
> @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
> pr_debug("Registering sysdev class '%s'\n",
> kobject_name(&cls->kset.kobj));
> INIT_LIST_HEAD(&cls->drivers);
> + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> cls->kset.kobj.parent = &system_kset->kobj;
> cls->kset.kobj.ktype = &ktype_sysdev_class;
> cls->kset.kobj.kset = system_kset;
> @@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
>
> pr_debug("Registering sys device '%s'\n",
> kobject_name(&sysdev->kobj));
>
> + memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
> /* Make sure the kset is set */
> sysdev->kobj.kset = &cls->kset;
>
Thanks, 2.6.25-rc4 + these two memset()s is finally stable for
me with no warnings, BUG()s, or panics.
(However, the patch is whitespace damaged with initial tabs
converted to spaces.)
If you want to pass this on to Linus, you can add a
Tested-by: Mikael Pettersson <[EMAIL PROTECTED]>
/Mikael
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sunday 09 March 2008 12:33:07 pm Greg KH wrote:
> > > > Hi,
> > > >
> > > > This patch does not fix it all! The problem is in fact more involved. I
> > > > also get these BUG reports when I reload
> > kvm-intel.
> > > >
> > > > BUG kmalloc-8: Object already free
> > > > [ 74.696570]
> > > > -
> > > > [ 74.696596]
> > > > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> > > > pid=1439
> > > > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559
> > > > cpu=3 pid=1439
> > > > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8
> > > > flags=0x1c3
> > > > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> > > >
> > > > This happens because, sysdev_class_register assigns a name to the
> > > > kobject, and kfrees the old name if any. The poisoned 'name' object
> > > > persists in case of statically allocated kobjects and as its passed to
> > > > kfree again when re registered, we get the above warning.
> > > >
> > > > So, AFAICS the best way to solve this is by fixing the kobject users
> > > > (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> > > > memset the kobject to zero before passing it to sysdev_register.
> > >
> > > I like the memset idea, how about this patch instead?
> > >
> > > thanks,
> > >
> > > greg k-h
> > >
> > > --- a/drivers/base/sys.c
> > > +++ b/drivers/base/sys.c
> > > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
> > > pr_debug("Registering sysdev class '%s'\n",
> > >kobject_name(&cls->kset.kobj));
> > > INIT_LIST_HEAD(&cls->drivers);
> > > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > > cls->kset.kobj.parent = &system_kset->kobj;
> > > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > > cls->kset.kobj.kset = system_kset;
> > >
> >
> > This should work.But I am afraid if we zeroed it, the kfree poison
> > test wouldn't catch even the genuine cases. Isn't it ? CMIIW.
>
> What "genuine cases"? kobjects should always be initialized to zero
> before they are registered.
>
The cases in which a freed kobject is passed to the register function ?
Probably they will be caught when do a memset..
> > A better fix according to me would be to zero the kobject in the
> > places where we know it is being re-registered (kvm, oprofile) etc.
> > This should do for now. But we should fix the sys_device users later,
> > for the next cycle.
>
> Are you sure you know all of the sysdev_class objects that can be
> re-registered?
>
Hmm.. Right. No, I only know that KVM and Oprofile use it.
> Can you test this patch out?
>
Yes the idea works. One more memset is needed in sysdev_register. Here's the
final patch.
diff --git a/drivers/base/sys.c b/drivers/base/sys.c
index 2f79c55..7c839d9 100644
--- a/drivers/base/sys.c
+++ b/drivers/base/sys.c
@@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_class * cls)
pr_debug("Registering sysdev class '%s'\n",
kobject_name(&cls->kset.kobj));
INIT_LIST_HEAD(&cls->drivers);
+ memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
cls->kset.kobj.parent = &system_kset->kobj;
cls->kset.kobj.ktype = &ktype_sysdev_class;
cls->kset.kobj.kset = system_kset;
@@ -227,6 +228,7 @@ int sysdev_register(struct sys_device * sysdev)
pr_debug("Registering sys device '%s'\n", kobject_name(&sysdev->kobj));
+ memset(&sysdev->kobj, 0x00, sizeof(struct kobject));
/* Make sure the kset is set */
sysdev->kobj.kset = &cls->kset;
--
regards,
balaji rao
3rd year,
Dept. of Mechanical Engineering,
National Institute of Technology, Karnataka
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sun, Mar 09, 2008 at 12:06:02PM +0530, Balaji Rao wrote:
> On Sunday 09 March 2008 12:03:08 pm Greg KH wrote:
> > On Sun, Mar 09, 2008 at 03:37:16AM +0530, Balaji Rao wrote:
> > > On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> > > > On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > > > > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> > > > >
> > > > > > Where exactly in the code does that happen? kobjects should not be
> > > > > > "reused" as that implies that they are static, and not dynamically
> > > > > > allocated, right?
> > > > > >
> > > > > > Which kobject is this?
> > > > > Yes, its static. Here's the code from virt/kvm_main.c:1269
> > > > >
> > > > > static struct sys_device kvm_sysdev = {
> > > > > .id = 0,
> > > > > .cls = &kvm_sysdev_class,
> > > > > };
> > > > >
> > > > > this sys_device is being registered/unregistered when kvm-intel is
> > > > > loaded/unloaded.
> > > >
> > > > Ah, ok. I'll add this patch then.
> > > >
> > > > > > Ugh, is this the sys_device stuff? I hate that code...
> > > > > >
> > > > > Yes it is! But, why do you hate it ?
> > > >
> > > > For reasons like this :)
> > > >
> > > > kobjects should not be static. the sysdevice stuff was a hack when it
> > > > was originally created and never touched since the mid 2.5 days. It
> > > > needs to be fixed up a lot, and is on my TODO list, slowly getting
> > > > closer to the top...
> > > Hi,
> > >
> > > This patch does not fix it all! The problem is in fact more involved. I
> > > also get these BUG reports when I reload
> kvm-intel.
> > >
> > > BUG kmalloc-8: Object already free
> > > [ 74.696570]
> > > -
> > > [ 74.696596]
> > > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> > > pid=1439
> > > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559
> > > cpu=3 pid=1439
> > > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
> > > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> > >
> > > This happens because, sysdev_class_register assigns a name to the
> > > kobject, and kfrees the old name if any. The poisoned 'name' object
> > > persists in case of statically allocated kobjects and as its passed to
> > > kfree again when re registered, we get the above warning.
> > >
> > > So, AFAICS the best way to solve this is by fixing the kobject users
> > > (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> > > memset the kobject to zero before passing it to sysdev_register.
> >
> > I like the memset idea, how about this patch instead?
> >
> > thanks,
> >
> > greg k-h
> >
> > --- a/drivers/base/sys.c
> > +++ b/drivers/base/sys.c
> > @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
> > pr_debug("Registering sysdev class '%s'\n",
> > kobject_name(&cls->kset.kobj));
> > INIT_LIST_HEAD(&cls->drivers);
> > + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> > cls->kset.kobj.parent = &system_kset->kobj;
> > cls->kset.kobj.ktype = &ktype_sysdev_class;
> > cls->kset.kobj.kset = system_kset;
> >
>
> This should work.But I am afraid if we zeroed it, the kfree poison
> test wouldn't catch even the genuine cases. Isn't it ? CMIIW.
What "genuine cases"? kobjects should always be initialized to zero
before they are registered.
> A better fix according to me would be to zero the kobject in the
> places where we know it is being re-registered (kvm, oprofile) etc.
> This should do for now. But we should fix the sys_device users later,
> for the next cycle.
Are you sure you know all of the sysdev_class objects that can be
re-registered?
Can you test this patch out?
thanks,
greg k-h
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sunday 09 March 2008 12:03:08 pm Greg KH wrote:
> On Sun, Mar 09, 2008 at 03:37:16AM +0530, Balaji Rao wrote:
> > On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> > > On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > > > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> > > >
> > > > > Where exactly in the code does that happen? kobjects should not be
> > > > > "reused" as that implies that they are static, and not dynamically
> > > > > allocated, right?
> > > > >
> > > > > Which kobject is this?
> > > > Yes, its static. Here's the code from virt/kvm_main.c:1269
> > > >
> > > > static struct sys_device kvm_sysdev = {
> > > > .id = 0,
> > > > .cls = &kvm_sysdev_class,
> > > > };
> > > >
> > > > this sys_device is being registered/unregistered when kvm-intel is
> > > > loaded/unloaded.
> > >
> > > Ah, ok. I'll add this patch then.
> > >
> > > > > Ugh, is this the sys_device stuff? I hate that code...
> > > > >
> > > > Yes it is! But, why do you hate it ?
> > >
> > > For reasons like this :)
> > >
> > > kobjects should not be static. the sysdevice stuff was a hack when it
> > > was originally created and never touched since the mid 2.5 days. It
> > > needs to be fixed up a lot, and is on my TODO list, slowly getting
> > > closer to the top...
> > Hi,
> >
> > This patch does not fix it all! The problem is in fact more involved. I
> > also get these BUG reports when I reload
kvm-intel.
> >
> > BUG kmalloc-8: Object already free
> > [ 74.696570]
> > -
> > [ 74.696596]
> > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> > pid=1439
> > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559
> > cpu=3 pid=1439
> > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
> > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> >
> > This happens because, sysdev_class_register assigns a name to the
> > kobject, and kfrees the old name if any. The poisoned 'name' object
> > persists in case of statically allocated kobjects and as its passed to
> > kfree again when re registered, we get the above warning.
> >
> > So, AFAICS the best way to solve this is by fixing the kobject users
> > (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> > memset the kobject to zero before passing it to sysdev_register.
>
> I like the memset idea, how about this patch instead?
>
> thanks,
>
> greg k-h
>
> --- a/drivers/base/sys.c
> +++ b/drivers/base/sys.c
> @@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
> pr_debug("Registering sysdev class '%s'\n",
>kobject_name(&cls->kset.kobj));
> INIT_LIST_HEAD(&cls->drivers);
> + memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
> cls->kset.kobj.parent = &system_kset->kobj;
> cls->kset.kobj.ktype = &ktype_sysdev_class;
> cls->kset.kobj.kset = system_kset;
>
This should work.But I am afraid if we zeroed it, the kfree poison test
wouldn't catch even the genuine cases. Isn't it ?
CMIIW.
A better fix according to me would be to zero the kobject in the places where
we know it is being re-registered (kvm,
oprofile) etc. This should do for now. But we should fix the sys_device users
later, for the next cycle.
--
regards,
balaji rao
3rd year,
Dept. of Mechanical Engineering,
National Institute of Technology, Karnataka
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sun, Mar 09, 2008 at 03:37:16AM +0530, Balaji Rao wrote:
> On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> > On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> > >
> > > > Where exactly in the code does that happen? kobjects should not be
> > > > "reused" as that implies that they are static, and not dynamically
> > > > allocated, right?
> > > >
> > > > Which kobject is this?
> > > Yes, its static. Here's the code from virt/kvm_main.c:1269
> > >
> > > static struct sys_device kvm_sysdev = {
> > > .id = 0,
> > > .cls = &kvm_sysdev_class,
> > > };
> > >
> > > this sys_device is being registered/unregistered when kvm-intel is
> > > loaded/unloaded.
> >
> > Ah, ok. I'll add this patch then.
> >
> > > > Ugh, is this the sys_device stuff? I hate that code...
> > > >
> > > Yes it is! But, why do you hate it ?
> >
> > For reasons like this :)
> >
> > kobjects should not be static. the sysdevice stuff was a hack when it
> > was originally created and never touched since the mid 2.5 days. It
> > needs to be fixed up a lot, and is on my TODO list, slowly getting
> > closer to the top...
> Hi,
>
> This patch does not fix it all! The problem is in fact more involved. I also
> get these BUG reports when I reload kvm-intel.
>
> BUG kmalloc-8: Object already free
> [ 74.696570]
> -
> [ 74.696596]
> [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> pid=1439
> [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559 cpu=3
> pid=1439
> [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
> [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
>
> This happens because, sysdev_class_register assigns a name to the
> kobject, and kfrees the old name if any. The poisoned 'name' object
> persists in case of statically allocated kobjects and as its passed to
> kfree again when re registered, we get the above warning.
>
> So, AFAICS the best way to solve this is by fixing the kobject users
> (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> memset the kobject to zero before passing it to sysdev_register.
I like the memset idea, how about this patch instead?
thanks,
greg k-h
--- a/drivers/base/sys.c
+++ b/drivers/base/sys.c
@@ -133,6 +133,7 @@ int sysdev_class_register(struct sysdev_
pr_debug("Registering sysdev class '%s'\n",
kobject_name(&cls->kset.kobj));
INIT_LIST_HEAD(&cls->drivers);
+ memset(&cls->kset.kobj, 0x00, sizeof(struct kobject));
cls->kset.kobj.parent = &system_kset->kobj;
cls->kset.kobj.ktype = &ktype_sysdev_class;
cls->kset.kobj.kset = system_kset;
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sunday 09 March 2008 10:36:49 am Greg KH wrote:
> On Sun, Mar 09, 2008 at 03:37:16AM +0530, Balaji Rao wrote:
> > On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> > > On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > > > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> > > >
> > > > > Where exactly in the code does that happen? kobjects should not be
> > > > > "reused" as that implies that they are static, and not dynamically
> > > > > allocated, right?
> > > > >
> > > > > Which kobject is this?
> > > > Yes, its static. Here's the code from virt/kvm_main.c:1269
> > > >
> > > > static struct sys_device kvm_sysdev = {
> > > > .id = 0,
> > > > .cls = &kvm_sysdev_class,
> > > > };
> > > >
> > > > this sys_device is being registered/unregistered when kvm-intel is
> > > > loaded/unloaded.
> > >
> > > Ah, ok. I'll add this patch then.
> > >
> > > > > Ugh, is this the sys_device stuff? I hate that code...
> > > > >
> > > > Yes it is! But, why do you hate it ?
> > >
> > > For reasons like this :)
> > >
> > > kobjects should not be static. the sysdevice stuff was a hack when it
> > > was originally created and never touched since the mid 2.5 days. It
> > > needs to be fixed up a lot, and is on my TODO list, slowly getting
> > > closer to the top...
> > Hi,
> >
> > This patch does not fix it all! The problem is in fact more involved.
> > I also get these BUG reports when I reload kvm-intel.
> >
> > BUG kmalloc-8: Object already free
> > [ 74.696570]
> > -
> > [ 74.696596]
> > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> > pid=1439
> > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559
> > cpu=3 pid=1439
> > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
> > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> >
> > This happens because, sysdev_class_register assigns a name to the
> > kobject, and kfrees the old name if any. The poisoned 'name' object
> > persists in case of statically allocated kobjects and as its passed to
> > kfree again when re registered, we get the above warning.
>
> Ugh, that's horrible. And people wonder why I hate the sysdev code :)
>
> Does it also mean that when we do cleanup sysdev devices, we are freeing
> a name that might not have been dynamically allocated? If so, we need
> to fix that as well.
>
No, that doesn't happen. We free the name only if it is allocated..
> A simple 'strdup' of the class name in sysdev_class_register() should
> fix all of this, right?
>
> > So, AFAICS the best way to solve this is by fixing the kobject users
> > (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> > memset the kobject to zero before passing it to sysdev_register.
>
> Yes, that would be great to fix up, but probably unlikly so late in the
> release cycle.
>
> How about the patch below, does it work for you? (build tested only)
>
> thanks,
>
> greg k-h
>
>
> ---
> drivers/base/sys.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> --- a/drivers/base/sys.c
> +++ b/drivers/base/sys.c
> @@ -130,13 +130,16 @@ static struct kset *system_kset;
>
> int sysdev_class_register(struct sysdev_class * cls)
> {
> + char *name;
> +
> pr_debug("Registering sysdev class '%s'\n",
>kobject_name(&cls->kset.kobj));
> INIT_LIST_HEAD(&cls->drivers);
> cls->kset.kobj.parent = &system_kset->kobj;
> cls->kset.kobj.ktype = &ktype_sysdev_class;
> cls->kset.kobj.kset = system_kset;
> - kobject_set_name(&cls->kset.kobj, cls->name);
> + name = kstrdup(cls->name, GFP_KERNEL);
> + kobject_set_name(&cls->kset.kobj, name);
> return kset_register(&cls->kset);
> }
>
>
--
regards,
balaji rao
3rd year,
Dept. of Mechanical Engineering,
National Institute of Technology, Karnataka
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sat, Mar 08, 2008 at 09:06:49PM -0800, Greg KH wrote:
> On Sun, Mar 09, 2008 at 03:37:16AM +0530, Balaji Rao wrote:
> > On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> > > On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > > > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> > > >
> > > > > Where exactly in the code does that happen? kobjects should not be
> > > > > "reused" as that implies that they are static, and not dynamically
> > > > > allocated, right?
> > > > >
> > > > > Which kobject is this?
> > > > Yes, its static. Here's the code from virt/kvm_main.c:1269
> > > >
> > > > static struct sys_device kvm_sysdev = {
> > > > .id = 0,
> > > > .cls = &kvm_sysdev_class,
> > > > };
> > > >
> > > > this sys_device is being registered/unregistered when kvm-intel is
> > > > loaded/unloaded.
> > >
> > > Ah, ok. I'll add this patch then.
> > >
> > > > > Ugh, is this the sys_device stuff? I hate that code...
> > > > >
> > > > Yes it is! But, why do you hate it ?
> > >
> > > For reasons like this :)
> > >
> > > kobjects should not be static. the sysdevice stuff was a hack when it
> > > was originally created and never touched since the mid 2.5 days. It
> > > needs to be fixed up a lot, and is on my TODO list, slowly getting
> > > closer to the top...
> > Hi,
> >
> > This patch does not fix it all! The problem is in fact more involved.
> > I also get these BUG reports when I reload kvm-intel.
> >
> > BUG kmalloc-8: Object already free
> > [ 74.696570]
> > -
> > [ 74.696596]
> > [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> > pid=1439
> > [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559
> > cpu=3 pid=1439
> > [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
> > [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
> >
> > This happens because, sysdev_class_register assigns a name to the
> > kobject, and kfrees the old name if any. The poisoned 'name' object
> > persists in case of statically allocated kobjects and as its passed to
> > kfree again when re registered, we get the above warning.
>
> Ugh, that's horrible. And people wonder why I hate the sysdev code :)
>
> Does it also mean that when we do cleanup sysdev devices, we are freeing
> a name that might not have been dynamically allocated? If so, we need
> to fix that as well.
>
> A simple 'strdup' of the class name in sysdev_class_register() should
> fix all of this, right?
Wait, no, that's not going to work, the name is already kstrduped. That
patch shouldn't do anything except leak memory.
Let me try something else...
thanks,
greg k-h
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Sun, Mar 09, 2008 at 03:37:16AM +0530, Balaji Rao wrote:
> On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> > On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> > >
> > > > Where exactly in the code does that happen? kobjects should not be
> > > > "reused" as that implies that they are static, and not dynamically
> > > > allocated, right?
> > > >
> > > > Which kobject is this?
> > > Yes, its static. Here's the code from virt/kvm_main.c:1269
> > >
> > > static struct sys_device kvm_sysdev = {
> > > .id = 0,
> > > .cls = &kvm_sysdev_class,
> > > };
> > >
> > > this sys_device is being registered/unregistered when kvm-intel is
> > > loaded/unloaded.
> >
> > Ah, ok. I'll add this patch then.
> >
> > > > Ugh, is this the sys_device stuff? I hate that code...
> > > >
> > > Yes it is! But, why do you hate it ?
> >
> > For reasons like this :)
> >
> > kobjects should not be static. the sysdevice stuff was a hack when it
> > was originally created and never touched since the mid 2.5 days. It
> > needs to be fixed up a lot, and is on my TODO list, slowly getting
> > closer to the top...
> Hi,
>
> This patch does not fix it all! The problem is in fact more involved.
> I also get these BUG reports when I reload kvm-intel.
>
> BUG kmalloc-8: Object already free
> [ 74.696570]
> -
> [ 74.696596]
> [ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2
> pid=1439
> [ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559 cpu=3
> pid=1439
> [ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
> [ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
>
> This happens because, sysdev_class_register assigns a name to the
> kobject, and kfrees the old name if any. The poisoned 'name' object
> persists in case of statically allocated kobjects and as its passed to
> kfree again when re registered, we get the above warning.
Ugh, that's horrible. And people wonder why I hate the sysdev code :)
Does it also mean that when we do cleanup sysdev devices, we are freeing
a name that might not have been dynamically allocated? If so, we need
to fix that as well.
A simple 'strdup' of the class name in sysdev_class_register() should
fix all of this, right?
> So, AFAICS the best way to solve this is by fixing the kobject users
> (kvm, oprofile etc.) to use dynamic kobjects instead of static ones or
> memset the kobject to zero before passing it to sysdev_register.
Yes, that would be great to fix up, but probably unlikly so late in the
release cycle.
How about the patch below, does it work for you? (build tested only)
thanks,
greg k-h
---
drivers/base/sys.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/drivers/base/sys.c
+++ b/drivers/base/sys.c
@@ -130,13 +130,16 @@ static struct kset *system_kset;
int sysdev_class_register(struct sysdev_class * cls)
{
+ char *name;
+
pr_debug("Registering sysdev class '%s'\n",
kobject_name(&cls->kset.kobj));
INIT_LIST_HEAD(&cls->drivers);
cls->kset.kobj.parent = &system_kset->kobj;
cls->kset.kobj.ktype = &ktype_sysdev_class;
cls->kset.kobj.kset = system_kset;
- kobject_set_name(&cls->kset.kobj, cls->name);
+ name = kstrdup(cls->name, GFP_KERNEL);
+ kobject_set_name(&cls->kset.kobj, name);
return kset_register(&cls->kset);
}
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Thursday 06 March 2008 11:35:59 pm Greg KH wrote:
> On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> > On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> >
> > > Where exactly in the code does that happen? kobjects should not be
> > > "reused" as that implies that they are static, and not dynamically
> > > allocated, right?
> > >
> > > Which kobject is this?
> > Yes, its static. Here's the code from virt/kvm_main.c:1269
> >
> > static struct sys_device kvm_sysdev = {
> > .id = 0,
> > .cls = &kvm_sysdev_class,
> > };
> >
> > this sys_device is being registered/unregistered when kvm-intel is
> > loaded/unloaded.
>
> Ah, ok. I'll add this patch then.
>
> > > Ugh, is this the sys_device stuff? I hate that code...
> > >
> > Yes it is! But, why do you hate it ?
>
> For reasons like this :)
>
> kobjects should not be static. the sysdevice stuff was a hack when it
> was originally created and never touched since the mid 2.5 days. It
> needs to be fixed up a lot, and is on my TODO list, slowly getting
> closer to the top...
Hi,
This patch does not fix it all! The problem is in fact more involved. I also
get these BUG reports when I reload kvm-intel.
BUG kmalloc-8: Object already free
[ 74.696570]
-
[ 74.696596]
[ 74.697310] INFO: Allocated in strndup_user+0x30/0x62 age=587 cpu=2 pid=1439
[ 74.697845] INFO: Freed in kobject_set_name_vargs+0x29/0x32 age=559 cpu=3
pid=1439
[ 74.698008] INFO: Slab 0xc16f93a0 used=10 fp=0xf7c9d2d8 flags=0x1c3
[ 74.698008] INFO: Object 0xf7c9d1f8 @offset=504 fp=0xf7c9d578
This happens because, sysdev_class_register assigns a name to the kobject, and
kfrees the old name if any. The
poisoned 'name' object persists in case of statically allocated kobjects and as
its passed to kfree again when re
registered, we get the above warning.
So, AFAICS the best way to solve this is by fixing the kobject users (kvm,
oprofile etc.) to use dynamic kobjects instead
of static ones or memset the kobject to zero before passing it to
sysdev_register.
Greg ?
--
regards,
balaji rao
3rd year,
Dept. of Mechanical Engineering,
National Institute of Technology, Karnataka
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
Greg KH wrote: > and is on my TODO list, slowly getting > closer to the top... > > Strange. On my TODO list, things slowly get pushed to the bottom. -- Any sufficiently difficult bug is indistinguishable from a feature. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Thu, Mar 06, 2008 at 11:20:50PM +0530, Balaji Rao wrote:
> On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
>
> > Where exactly in the code does that happen? kobjects should not be
> > "reused" as that implies that they are static, and not dynamically
> > allocated, right?
> >
> > Which kobject is this?
> Yes, its static. Here's the code from virt/kvm_main.c:1269
>
> static struct sys_device kvm_sysdev = {
> .id = 0,
> .cls = &kvm_sysdev_class,
> };
>
> this sys_device is being registered/unregistered when kvm-intel is
> loaded/unloaded.
Ah, ok. I'll add this patch then.
> > Ugh, is this the sys_device stuff? I hate that code...
> >
> Yes it is! But, why do you hate it ?
For reasons like this :)
kobjects should not be static. the sysdevice stuff was a hack when it
was originally created and never touched since the mid 2.5 days. It
needs to be fixed up a lot, and is on my TODO list, slowly getting
closer to the top...
thanks,
greg k-h
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Thursday 06 March 2008 10:35:14 pm Greg KH wrote:
> Where exactly in the code does that happen? kobjects should not be
> "reused" as that implies that they are static, and not dynamically
> allocated, right?
>
> Which kobject is this?
Yes, its static. Here's the code from virt/kvm_main.c:1269
static struct sys_device kvm_sysdev = {
.id = 0,
.cls = &kvm_sysdev_class,
};
this sys_device is being registered/unregistered when kvm-intel is
loaded/unloaded.
>
> Ugh, is this the sys_device stuff? I hate that code...
>
Yes it is! But, why do you hate it ?
--
regards,
balaji rao
3rd year,
Dept. of Mechanical Engineering,
National Institute of Technology, Karnataka
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Mark kobjects as unitialized
On Thu, Mar 06, 2008 at 10:23:18PM +0530, Balaji Rao wrote: > Hi greg, > > When I remove only the kvm-intel module without removing the kvm > module itself, I get an error saying that a kobject is trying to be > reinitialized. Its because of the fact that kvm reuses a kobject in > kvm_init when calling sysdev_register. Where exactly in the code does that happen? kobjects should not be "reused" as that implies that they are static, and not dynamically allocated, right? Which kobject is this? Ugh, is this the sys_device stuff? I hate that code... > This patch fixes kobject_cleanup by marking the kobject as > uninitialized when we cleanup to allow kobjects to be reused. The patch seems correct, but I would like to verify which kobject is being reused here first. thanks, greg k-h - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
