[asterisk-dev] [Code Review] res_srtp: Fix a crash caused by an attempt to dealloc a session pointer that was never alloced or has already been dealloced.

[Matt Jordan]

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



/branches/1.8/res/res_srtp.c
<https://reviewboard.asterisk.org/r/2228/#comment14384>

    I'm not a fan of variables in an inner block scope hiding variables in outer blocks. In this case, the declaration of res here hides the previous declaration of res at the beginning of the function. In general, it this can make the code more difficult to maintain.
    
    Unfortunately, as you probably noticed, re-using res here is also not advisable since the original value is needed outside of this scope. You're probably better off just making a new variable (res_create?).


- Matt


On Dec. 20, 2012, 10:32 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2228/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2012, 10:32 a.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and Matt Jordan.
> 
> 
> Summary
> -------
> 
> If srtp_create fails, session we create for temp will either not be created or else will be dealloced within srtp_create. At present, when we then run ast_srtp_destroy, attempting to dealloc this can cause Asterisk to crash. This patch addresses that by setting the session pointer to NULL so ast_srtp_destroy doesn't attempt to dealloc it.
> 
> As one might expect, this patch doesn't resolve the reporter's problems with actually setting up an srtp enabled SIP call, but it does fix a crash which shouldn't be happening.
> 
> 
> This addresses bug ASTERISK-20499.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20499
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/res/res_srtp.c 378146 
> 
> Diff: https://reviewboard.asterisk.org/r/2228/diff
> 
> 
> Testing
> -------
> 
> The reporter has tested the patch and confirmed that it eliminates the crash.
> 
> 
> Thanks,
> 
> jrose
> 
>

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