2009-04-11 16:16:39

by Ming Lei

Subject: [PATCH] kobject: kzfree object in _release function

From: Ming Lei <[email protected]>

It helps to troubleshoot the __buggy__ case, in which
unreferenced objects are still accessed, using kzfree
to free objects safely in _release function.

Signed-off-by: Ming Lei <[email protected]>
---
lib/kobject.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index a6dec32..cb5a562 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -597,7 +597,7 @@ void kobject_put(struct kobject *kobj)
static void dynamic_kobj_release(struct kobject *kobj)
{
pr_debug("kobject: (%p): %s\n", kobj, __func__);
- kfree(kobj);
+ kzfree(kobj);
}

static struct kobj_type dynamic_kobj_ktype = {
@@ -762,7 +762,7 @@ static void kset_release(struct kobject *kobj)
struct kset *kset = container_of(kobj, struct kset, kobj);
pr_debug("kobject: '%s' (%p): %s\n",
kobject_name(kobj), kobj, __func__);
- kfree(kset);
+ kzfree(kset);
}

static struct kobj_type kset_ktype = {
--
1.6.0.GIT

2009-04-13 16:36:50

by Greg KH

Subject: Re: [PATCH] kobject: kzfree object in _release function

On Sun, Apr 12, 2009 at 12:16:26AM +0800, [email protected] wrote:
> From: Ming Lei <[email protected]>
>
> It helps to troubleshoot the __buggy__ case, in which
> unreferenced objects are still accessed, using kzfree
> to free objects safely in _release function.
>
> Signed-off-by: Ming Lei <[email protected]>
> ---
> lib/kobject.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/kobject.c b/lib/kobject.c
> index a6dec32..cb5a562 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -597,7 +597,7 @@ void kobject_put(struct kobject *kobj)
> static void dynamic_kobj_release(struct kobject *kobj)
> {
> pr_debug("kobject: (%p): %s\n", kobj, __func__);
> - kfree(kobj);
> + kzfree(kobj);
> }
>
> static struct kobj_type dynamic_kobj_ktype = {
> @@ -762,7 +762,7 @@ static void kset_release(struct kobject *kobj)
> struct kset *kset = container_of(kobj, struct kset, kobj);
> pr_debug("kobject: '%s' (%p): %s\n",
> kobject_name(kobj), kobj, __func__);
> - kfree(kset);
> + kzfree(kset);


What about slab-poisoning? Shouldn't we just rely on that instead?

thanks,

greg k-h

2009-04-13 16:38:00

by Pekka Enberg

Subject: Re: [PATCH] kobject: kzfree object in _release function

Hi Greg,

On Mon, Apr 13, 2009 at 7:31 PM, Greg KH <[email protected]> wrote:
> On Sun, Apr 12, 2009 at 12:16:26AM +0800, [email protected] wrote:
>> From: Ming Lei <[email protected]>
>>
>> It helps to troubleshoot the __buggy__ case, in which
>> unreferenced objects are still accessed, using kzfree
>> to free objects safely in _release function.
>>
>> Signed-off-by: Ming Lei <[email protected]>
>> ---
>> ?lib/kobject.c | ? ?4 ++--
>> ?1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/kobject.c b/lib/kobject.c
>> index a6dec32..cb5a562 100644
>> --- a/lib/kobject.c
>> +++ b/lib/kobject.c
>> @@ -597,7 +597,7 @@ void kobject_put(struct kobject *kobj)
>> ?static void dynamic_kobj_release(struct kobject *kobj)
>> ?{
>> ? ? ? pr_debug("kobject: (%p): %s\n", kobj, __func__);
>> - ? ? kfree(kobj);
>> + ? ? kzfree(kobj);
>> ?}
>>
>> ?static struct kobj_type dynamic_kobj_ktype = {
>> @@ -762,7 +762,7 @@ static void kset_release(struct kobject *kobj)
>> ? ? ? struct kset *kset = container_of(kobj, struct kset, kobj);
>> ? ? ? pr_debug("kobject: '%s' (%p): %s\n",
>> ? ? ? ? ? ? ? ?kobject_name(kobj), kobj, __func__);
>> - ? ? kfree(kset);
>> + ? ? kzfree(kset);
>
>
> What about slab-poisoning? ?Shouldn't we just rely on that instead?

Yes, you should. kzfree() strictly for cases where _correctness_ of
the program requires it (i.e. security concerns).

2009-04-13 20:36:18

by Greg KH

Subject: Re: [PATCH] kobject: kzfree object in _release function

On Mon, Apr 13, 2009 at 07:37:47PM +0300, Pekka Enberg wrote:
> Hi Greg,
>
> On Mon, Apr 13, 2009 at 7:31 PM, Greg KH <[email protected]> wrote:
> > On Sun, Apr 12, 2009 at 12:16:26AM +0800, [email protected] wrote:
> >> From: Ming Lei <[email protected]>
> >>
> >> It helps to troubleshoot the __buggy__ case, in which
> >> unreferenced objects are still accessed, using kzfree
> >> to free objects safely in _release function.
> >>
> >> Signed-off-by: Ming Lei <[email protected]>
> >> ---
> >> ?lib/kobject.c | ? ?4 ++--
> >> ?1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/kobject.c b/lib/kobject.c
> >> index a6dec32..cb5a562 100644
> >> --- a/lib/kobject.c
> >> +++ b/lib/kobject.c
> >> @@ -597,7 +597,7 @@ void kobject_put(struct kobject *kobj)
> >> ?static void dynamic_kobj_release(struct kobject *kobj)
> >> ?{
> >> ? ? ? pr_debug("kobject: (%p): %s\n", kobj, __func__);
> >> - ? ? kfree(kobj);
> >> + ? ? kzfree(kobj);
> >> ?}
> >>
> >> ?static struct kobj_type dynamic_kobj_ktype = {
> >> @@ -762,7 +762,7 @@ static void kset_release(struct kobject *kobj)
> >> ? ? ? struct kset *kset = container_of(kobj, struct kset, kobj);
> >> ? ? ? pr_debug("kobject: '%s' (%p): %s\n",
> >> ? ? ? ? ? ? ? ?kobject_name(kobj), kobj, __func__);
> >> - ? ? kfree(kset);
> >> + ? ? kzfree(kset);
> >
> >
> > What about slab-poisoning? ?Shouldn't we just rely on that instead?
>
> Yes, you should. kzfree() strictly for cases where _correctness_ of
> the program requires it (i.e. security concerns).

Thanks for letting us know. As such, I'll not be applying this patch.

greg k-h