[asterisk-dev] [Code Review]: Fix memory leak of SSL_CTX

[Mark Michelson]

> On April 18, 2012, 11:55 a.m., Matt Jordan wrote:
> > /branches/1.8/main/tcptls.c, line 355
> > <https://reviewboard.asterisk.org/r/1875/diff/1/?file=27396#file27396line355>
> >
> >     Is it just me, or do these sleep(2) calls seem odd?

It's definitely not just you. I honestly have no idea what purpose they could serve.


> On April 18, 2012, 11:55 a.m., Matt Jordan wrote:
> > /branches/1.8/main/tcptls.c, line 317
> > <https://reviewboard.asterisk.org/r/1875/diff/1/?file=27396#file27396line317>
> >
> >     Currently, the only way we'll dispose ssl_ctx is if we call __ssl_setup on an ast_tls_config object twice.  This means that we would continue to leak the last allocated ssl_ctx object.
> >     
> >     This is problematic, as ast_tls_config objects can be initialized either when they are associated with an ast_tcptls_session_instance, or by themselves via ast_ssl_setup.
> >     
> >     When its associated with an ast_tcptls_session_instance, the ssl_ctx (if allocated) can be disposed of in that object's ao2 destructor.
> >     
> >     However, when the config object exists by itself (which it does in chan_sip's reload_config), we probably need a corresponding method to ast_ssl_setup that tears down the configuration object and disposes of the context object appropriately.
> >     
> >

Yep, looks like you're right there. I'll comb through the users of the TCP/TLS API in addition to chan_sip and make sure they also clean up after themselves.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1875/#review6014
-----------------------------------------------------------


On April 18, 2012, 10:54 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1875/
> -----------------------------------------------------------
> 
> (Updated April 18, 2012, 10:54 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Based on information-gathering in issue ASTERISK-19278, it appears that Asterisk is misusing OpenSSL in a way that causes a slow memory leak. From combing through main/tcptls.c, it appears that one foul-up Asterisk has is to never free any SSL_CTXs that are created. I have patched __ssl_setup() to properly free the SSL_CTX if either
> 
> 1) We're about to allocate a new one.
> 2) Some failure occurs during setup.
> 
> While I have no confirmation that this is the same memory leak being observed in ASTERISK-19278, this fix seems necessary anyway. I'm placing this patch on Review Board because my unfamiliarity with OpenSSL leads me to seek confirmation that what I am doing makes sense and will not cause issues.
> 
> 
> This addresses bug ASTERISK-19278.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19278
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/tcptls.c 362427 
> 
> Diff: https://reviewboard.asterisk.org/r/1875/diff
> 
> 
> Testing
> -------
> 
> No testing has been done aside from compilation. I will ask the reporter on ASTERISK-19278 to test to see if the memory leak appears to go away.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120418/4561779f/attachment.htm>