lkml.org 
[lkml]   [2009]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] kobject_set_name_vargs memory leak
    On (06/26/09 16:00), Eric W. Biederman wrote:
    > >> > 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
    >
    Or work with 'new_name' and overwrite kobject->name only 'if(new_name)'.
    I thought about restoring. ( Blue or Red Pill? :) )

    diff --git a/lib/kobject.c b/lib/kobject.c
    index b512b74..d6b1502 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, '/')))


    Sergey


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