lkml.org 
[lkml]   [2009]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] kobject_set_name_vargs memory leak
    Sergey Senozhatsky <sergey.senozhatsky@mail.by> writes:

    > On (06/26/09 07:49), Greg KH wrote:
    >> Date: Fri, 26 Jun 2009 07:49:49 -0700
    >> From: Greg KH <gregkh@suse.de>
    >> To: Sergey Senozhatsky <sergey.senozhatsky@mail.by>
    >> Cc: Kay Sievers <kay.sievers@vrfy.org>,
    >> "Eric W. Biederman" <ebiederm@xmission.com>,
    >> linux-kernel@vger.kernel.org
    >> Subject: Re: [PATCH] kobject_set_name_vargs memory leak
    >> User-Agent: Mutt/1.5.19 (2009-01-05)
    >>
    >> On Fri, Jun 26, 2009 at 05:36:52PM +0300, Sergey Senozhatsky wrote:
    >> > Hello.
    >> > I suppose this patch fixes memory leak in kobject.c
    >> > Correct me if I'm wrong.
    >> > Thanks.
    >> > -----------
    >> >
    >> > Fix memory leak when kobject_set_name_vargs returns -ENOMEM.
    >> >
    >> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@mail.by>
    >> > ---
    >> > diff --git a/lib/kobject.c b/lib/kobject.c
    >> > index b512b74..922cd8c 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) {
    >> > + kfree(old_name);
    >> > return -ENOMEM;
    >> > + }
    >>
    >> 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.
    >>
    > Hello Greg,
    >
    > int kobject_set_name_vargs.... {
    > const char *old_name = kobj->name;
    >
    > old_name is local variable.
    >
    > In the following lines we overwrite kobject->name.
    >
    > kobj->name = kvasprintf(GFP_KERNEL, fmt, vargs);
    > if (!kobj->name)
    > return -ENOMEM;
    >
    > It's not clear to me how we can do anything (including kfree) with old_name after 'return -ENOMEM'.

    My feel is that if we fail we should restore kobject->name to old_name.

    That should also prevent the leak without getting us into trouble elsewhere.

    Eric



    \
     
     \ /
      Last update: 2009-06-27 01:03    [W:0.027 / U:31.204 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site