ngx_resolver.c leaks memory?

Maxim Dounin mdounin at mdounin.ru
Thu Sep 17 14:40:23 MSD 2009


Hello!

On Wed, Sep 16, 2009 at 09:24:33AM -0700, Matthew Dempsky wrote:

> On Wed, Sep 16, 2009 at 3:35 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> > Patch looks correct for me.  It looks a bit fragile though,
> > probably we need a bit more bulletproof code here.
> 
> Would you mind elaborating on what you think is fragile about it?
> E.g., how would you rather this bug be patched?

I personally prefer something that won't explode once "goto 
failed;" will be reused somewhere after first ngx_resolver_free(r, 
name->data).  Just adding "name->data = NULL;" should be enough 
(though it's redundant right now).

> > More generally - resolver known to leak, and probably requires
> > code audit.  It would be fine if you look into it.  I believe
> > Artem Bokhan will help with testing (cc'd as I'm not sure he is on
> > English list).
> 
> Great.  I'll spend some time looking at the rest of the code then.

BTW, it looks like CNAME case in the same function will also leak 
as it sets rn->u.cname without freeing it first.

Probably we just need something like allocation pools as used in 
other parts of nginx code.  No idea why Igor wasn't used them in 
resolver.  Igor, could you please explain reasons?

Maxim Dounin





More information about the nginx mailing list