[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.

[jrose]

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

(Updated Dec. 6, 2012, 11:02 a.m.)


Review request for Asterisk Developers, Mark Michelson and Matt Jordan.


Changes
-------

The following patch alleviates the unconditional crash introduced by setting the session to NULL. It does so by changing the errno which will be received by the reading function when the srtp session has been NULLed from EAGAIN to EINVAL. EINVAL might not be the appropriate value to use here. I chose it since it this occurring probably means something in the policy was improperly set which is part of the srtp parameter.

I don't have a great deal of confidence in this patch, but considering the way this function was cobbled together I doubt I'd have confidence in any patch.


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 (updated)
-----

  /branches/1.8/res/res_srtp.c 377339 

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/20121206/4e2f7b4e/attachment-0001.htm>