Re: [PATCH] kobject_set_name_vargs memory leak

[Sergey Senozhatsky]

On (06/26/09 19:10), Eric W. Biederman wrote:
> Date: Fri, 26 Jun 2009 19:10:08 -0700
> From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> To: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxx>
> Cc: Greg KH <gregkh@xxxxxxx>, Kay Sievers <kay.sievers@xxxxxxxx>,
> 	linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] kobject_set_name_vargs memory leak
> User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux)
> 
> Sergey Senozhatsky <sergey.senozhatsky@xxxxxxx> writes:
> 
> > On (06/26/09 07:49), Greg KH wrote:
> >> We've been through this before (search lkml archives).  If kvasprintf
> >> fails, then we don't want to free old_name, as the caller might want to
> >> do something with it.
> >> 
> >> Or something along those lines, I can't remember the exact reasoning
> >> this early in the morning.
> >> 
> >> Kay, do you remember?
> >>
> > I found.
> > http://lkml.org/lkml/2009/5/11/11
> >
> >>kobject with name set before should not come into this function,
> >>kobject_rename should be used instead.
> >
> > It's just would be safer to kfree or restore I guess.
> 
> Yes.  There does seem to be a good point in there that the code should be:
> BUG_ON(kobj->name);
> 
> And otherwise simply not handle old_name at all.
> 

Sorry, correct one.

diff --git a/lib/kobject.c b/lib/kobject.c
index b512b74..3ab224b 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -215,7 +215,6 @@ static int kobject_add_internal(struct kobject *kobj)
 int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 				  va_list vargs)
 {
-	const char *old_name = kobj->name;
 	char *s;
 
 	if (kobj->name && !fmt)
@@ -229,7 +228,6 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 	while ((s = strchr(kobj->name, '/')))
 		s[0] = '!';
 
-	kfree(old_name);
 	return 0;
 }
 

> Eric
> 

	Sergey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/