[Asterisk-code-review] res rtp asterisk.c: Fix crash in RTCP DTLS operation. (asterisk[master])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5505 )

Change subject: res_rtp_asterisk.c: Fix crash in RTCP DTLS operation.
......................................................................


res_rtp_asterisk.c: Fix crash in RTCP DTLS operation.

Occasionally a crash happens when processing the RTCP DTLS timeout
handler.  The RTCP DTLS timeout timer could be left running if we have not
completed the DTLS handshake before we place the call on hold or we
attempt direct media.

* Made ast_rtp_prop_set() stop the RTCP DTLS timer when disabling RTCP.

* Made some sanity tweaks to ast_rtp_prop_set() when switching from
standard RTCP mode to RTCP multiplexed mode.

ASTERISK-26692 #close

Change-Id: If6c64c79129961acfa4b3d63a864e8f6b664acc0
---
M res/res_rtp_asterisk.c
1 file changed, 10 insertions(+), 8 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Jenkins2: Approved for Submit
  Matthew Fredrickson: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index e263832..d0d7959 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -1721,8 +1721,10 @@
 		dtls_srtp_stop_timeout_timer(instance, rtp, 1);
 		ao2_lock(instance);
 
-		if (rtp->rtcp->dtls.ssl && (rtp->rtcp->dtls.ssl != ssl)) {
-			SSL_free(rtp->rtcp->dtls.ssl);
+		if (rtp->rtcp->dtls.ssl) {
+			if (rtp->rtcp->dtls.ssl != ssl) {
+				SSL_free(rtp->rtcp->dtls.ssl);
+			}
 			rtp->rtcp->dtls.ssl = NULL;
 		}
 	}
@@ -5445,14 +5447,14 @@
 				 * to activating RTP. It is not until RTP is activated that timers start for RTCP
 				 * transmission
 				 */
-				if (rtp->rtcp->s > -1) {
+				if (rtp->rtcp->s > -1 && rtp->rtcp->s != rtp->s) {
 					close(rtp->rtcp->s);
 				}
 				rtp->rtcp->s = rtp->s;
 				ast_rtp_instance_get_remote_address(instance, &addr);
 				ast_sockaddr_copy(&rtp->rtcp->them, &addr);
 #ifdef HAVE_OPENSSL_SRTP
-				if (rtp->rtcp->dtls.ssl) {
+				if (rtp->rtcp->dtls.ssl && rtp->rtcp->dtls.ssl != rtp->dtls.ssl) {
 					SSL_free(rtp->rtcp->dtls.ssl);
 				}
 				rtp->rtcp->dtls.ssl = rtp->dtls.ssl;
@@ -5460,7 +5462,6 @@
 			}
 
 			ast_debug(1, "Setup RTCP on RTP instance '%p'\n", instance);
-			return;
 		} else {
 			if (rtp->rtcp) {
 				if (rtp->rtcp->schedid > -1) {
@@ -5481,6 +5482,10 @@
 					close(rtp->rtcp->s);
 				}
 #ifdef HAVE_OPENSSL_SRTP
+				ao2_unlock(instance);
+				dtls_srtp_stop_timeout_timer(instance, rtp, 1);
+				ao2_lock(instance);
+
 				if (rtp->rtcp->dtls.ssl && rtp->rtcp->dtls.ssl != rtp->dtls.ssl) {
 					SSL_free(rtp->rtcp->dtls.ssl);
 				}
@@ -5489,11 +5494,8 @@
 				ast_free(rtp->rtcp);
 				rtp->rtcp = NULL;
 			}
-			return;
 		}
 	}
-
-	return;
 }
 
 /*! \pre instance is locked */

-- 
To view, visit https://gerrit.asterisk.org/5505
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If6c64c79129961acfa4b3d63a864e8f6b664acc0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>