#361 Possible memleak in pseudo variables code

closed-fixed
core (125)
3
2008-05-03
2007-12-21
Dan Pascu
No

The functions pv_parse_hdr_name, pv_parse_avp_name and pv_parse_index allocate a nsp variable and after that call pv_parse_spec. If pv_parse_spec is not successful, the functions return without freeing nsp. I'm not sure if pv_parse_spec retains a reference to nsp, but if not there is clearly a mem leak.

Discussion

  • Salahuddin Ahmed

    Logged In: YES
    user_id=1953307
    Originator: NO

    Possible solution of this memleak:
    patch:
    ===========================================================
    --- pvar.c 2007-12-17 16:41:29.000000000 +0600
    +++ pvar_my_patch.c 2007-12-23 14:12:01.000000000 +0600
    @@ -2225,6 +2225,7 @@
    if(p==NULL)
    {
    LM_ERR("invalid name [%.*s]\n", in->len, in->s);
    + pkg_free(nsp);
    return -1;
    }
    //LM_ERR("dynamic name [%.*s]\n", in->len, in->s);
    @@ -2286,6 +2287,7 @@
    if(s==NULL)
    {
    LM_ERR("invalid name [%.*s]\n", in->len, in->s);
    + pkg_free(nsp);
    return -1;
    }
    //LM_ERR("dynamic name [%.*s]\n", in->len, in->s);
    @@ -2327,6 +2329,7 @@
    if(s==NULL)
    {
    LM_ERR("invalid index [%.*s]\n", in->len, in->s);
    + pkg_free(nsp);
    return -1;
    }
    sp->pvp.pvi.type = PV_IDX_PVAR;
    ===========================================================

    Regards,
    bd [dot] rubel [at] gmail [dot] com

     
  • Dan Pascu

    Dan Pascu - 2007-12-28

    Logged In: YES
    user_id=1296758
    Originator: YES

    I think we all know how a fix should look if it was to be applied. My question was more about the pertinence of such a fix. This is why I think that someone with in depth knowledge of the code should take a look and check if it is not retained somewhere else and still needed even after those functions return an error.

     
  • Salahuddin Ahmed

    Logged In: YES
    user_id=1953307
    Originator: NO

    Sorry for my work. Actually I was not understand the actual problem. Actually if this memory retains in this function pv_parse_spec and free later then it was not a memory leak. Sorry, I was not understand this.

     
  • Daniel-Constantin Mierla

    Logged In: YES
    user_id=1246013
    Originator: NO

    Indeed, free mem seems to be missing. It should not be a runtime leak as those functions are used at startup and when returning error, openser won't start. However, a fix should be in place, for a nice error handling, I will take care of it.

     
  • Daniel-Constantin Mierla

    • assigned_to: nobody --> miconda
     
  • Daniel-Constantin Mierla

    Logged In: YES
    user_id=1246013
    Originator: NO

    Used designated function to free the spec pv_spec_free() in the SVN commit. As it still needs some more work to do it clean, I will keep it open -- once again, it is harmless as when such error occurs, openser won't start.

     
  • Daniel-Constantin Mierla

    • priority: 5 --> 3
     
  • Daniel-Constantin Mierla

    Logged In: YES
    user_id=1246013
    Originator: NO

    Transformations are also cleaned in this case.

     
  • Daniel-Constantin Mierla

    • status: open --> closed-fixed
     

Log in to post a comment.