[PATCH] kobject: name reference should not be lost when lack of memory error happens

[PATCH] kobject: name reference should not be lost when lack of memory error happens

[Roman Tereshonkov]

If lack of memory error happens we must restore the old kobject name reference

Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com>
---
 lib/kobject.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index f07c572..c5cb117 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -222,8 +222,10 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 		return 0;
 
 	kobj->name = kvasprintf(GFP_KERNEL, fmt, vargs);
-	if (!kobj->name)
+	if (!kobj->name) {
+		kobj->name = old_name;
 		return -ENOMEM;
+	}
 
 	/* ewww... some of these buggers have '/' in the name ... */
 	while ((s = strchr(kobj->name, '/')))
-- 
1.7.0.4

Re: [PATCH] kobject: name reference should not be lost when lack of memory error happens

[Greg KH]

On Thu, Oct 07, 2010 at 01:32:53PM +0300, Roman Tereshonkov wrote:
> If lack of memory error happens we must restore the old kobject name reference

No, we can't do that, as the caller will not be expecting that to have
happened, right Kay?

I think the code as-is is correct, your kobject is in an unknown state
and can't be used anymore.

thanks,

greg k-h

> Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com>
> ---
>  lib/kobject.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/kobject.c b/lib/kobject.c
> index f07c572..c5cb117 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -222,8 +222,10 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
>  		return 0;
>  
>  	kobj->name = kvasprintf(GFP_KERNEL, fmt, vargs);
> -	if (!kobj->name)
> +	if (!kobj->name) {
> +		kobj->name = old_name;
>  		return -ENOMEM;
> +	}
>  
>  	/* ewww... some of these buggers have '/' in the name ... */
>  	while ((s = strchr(kobj->name, '/')))
> -- 
> 1.7.0.4

Re: [PATCH] kobject: name reference should not be lost when lack of memory error happens

[Kay Sievers]

On Thu, Oct 7, 2010 at 19:53, Greg KH <greg@kroah.com> wrote:
> On Thu, Oct 07, 2010 at 01:32:53PM +0300, Roman Tereshonkov wrote:
>> If lack of memory error happens we must restore the old kobject name reference
>
> No, we can't do that, as the caller will not be expecting that to have
> happened, right Kay?
>
> I think the code as-is is correct, your kobject is in an unknown state
> and can't be used anymore.

Yeah, we've been there several times already with no clear decision so far.

Trying to set a kobject name twice seems like a serious bug.

We should kill the old_name logic, and add a WARN() when kobj->name is
already set when the function is called, I think.

People must use kobject_rename() if they want a new name, like the
comment above kobject_set_name() says.

Kay

Re: [PATCH] kobject: name reference should not be lost when lack of memory error happens

[Greg KH]

On Sat, Oct 09, 2010 at 04:32:44PM +0200, Kay Sievers wrote:
> On Thu, Oct 7, 2010 at 19:53, Greg KH <greg@kroah.com> wrote:
> > On Thu, Oct 07, 2010 at 01:32:53PM +0300, Roman Tereshonkov wrote:
> >> If lack of memory error happens we must restore the old kobject name reference
> >
> > No, we can't do that, as the caller will not be expecting that to have
> > happened, right Kay?
> >
> > I think the code as-is is correct, your kobject is in an unknown state
> > and can't be used anymore.
> 
> Yeah, we've been there several times already with no clear decision so far.
> 
> Trying to set a kobject name twice seems like a serious bug.
> 
> We should kill the old_name logic, and add a WARN() when kobj->name is
> already set when the function is called, I think.

Agreed, it will be interesting to see what is trying to do this :)

thanks,

greg k-h