Merged revisions 378593 via svnmerge from

file:///srv/subversion/repos/asterisk/trunk

................
  r378593 | jrose | 2013-01-04 17:14:54 -0600 (Fri, 04 Jan 2013) | 23 lines

  res_srtp: Prevent a crash from occurring due to srtp_create failures in srtp_create

  Under some circumstances, libsrtp's srtp_create function deallocates memory that
  it wasn't initially responsible for allocating. Because we weren't initially
  aware of this behavior, this memory was still used in spite of being unallocated
  during the course of the srtp_unprotect function. A while back I made a patch
  which would set this value to NULL, but that exposed a possible condition where
  we would then try to check a member of the struct which would cause a segfault.
  In order to address these problems, ast_srtp_unprotect will now set an error value
  when it ends without a valid SRTP session which will result in the caller of
  srtp_unprotect observing this error and hanging up the relevant channel instead of
  trying to keep using the invalid session address.

  (closes issue ASTERISK-20499)
  Reported by: Tootai
  Review: https://reviewboard.asterisk.org/r/2228/diff/#index_header
  ........

  Merged revisions 378591 from http://svn.asterisk.org/svn/asterisk/branches/1.8
  ........

  Merged revisions 378592 from http://svn.asterisk.org/svn/asterisk/branches/11
................

git-svn-id: https://origsvn.digium.com/svn/asterisk/team/mmichelson/threadpool@378600 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/res/res_srtp.c b/res/res_srtp.c
index b9499f8f1c7da54b5ece833833d5e71b8267838f..549a1eaeb9514c3f56d0ea3768a4fd67650ed168 100644 (file)
--- a/res/res_srtp.c
+++ b/res/res_srtp.c
@@ -366,7 +366,8 @@ tryagain:
 
                        ast_debug(5, "SRTP try to re-create\n");
                        if (policy) {
-                               if (srtp_create(&srtp->session, &policy->sp) == err_status_ok) {
+                               int res_srtp_create = srtp_create(&srtp->session, &policy->sp);
+                               if (res_srtp_create == err_status_ok) {
                                        ast_debug(5, "SRTP re-created with first policy\n");
                                        ao2_t_ref(policy, -1, "Unreffing first policy for re-creating srtp session");
 
@@ -383,15 +384,23 @@ tryagain:
                                        retry++;
                                        ao2_iterator_destroy(&it);
                                        goto tryagain;
-                               } else {
-                                       srtp->session = NULL;
                                }
+                               ast_log(LOG_ERROR, "SRTP session could not be re-created after unprotect failure: %s\n", srtp_errstr(res_srtp_create));
+
+                               /* If srtp_create() fails with a previously alloced session, it will have been dealloced before returning. */
+                               srtp->session = NULL;
+
                                ao2_t_ref(policy, -1, "Unreffing first policy after srtp_create failed");
                        }
                        ao2_iterator_destroy(&it);
                }
        }
 
+       if (!srtp->session) {
+               errno = EINVAL;
+               return -1;
+       }
+
        if (res != err_status_ok && res != err_status_replay_fail ) {
                if ((srtp->warned >= 10) && !((srtp->warned - 10) % 100)) {
                        ast_log(AST_LOG_WARNING, "SRTP unprotect failed with: %s %d\n", srtp_errstr(res), srtp->warned);