[Asterisk-code-review] res_rtp_asterisk: Re-order RTP destruction. (asterisk[13])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14450 )

Change subject: res_rtp_asterisk: Re-order RTP destruction.
......................................................................

res_rtp_asterisk: Re-order RTP destruction.

The destructor for RTP deallocated transport resources
before terminating the ICE support. This could result
in a crash as the thread handling ICE would access already
freed parts of the RTP data.

This change re-orders the destruction so that ICE is
stopped before destroying things.

ASTERISK-28885

Change-Id: Ie71add549f12b6cdea7e9dbf976d1bd1d2fc0bdc
---
M res/res_rtp_asterisk.c
1 file changed, 33 insertions(+), 33 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved
  George Joseph: Looks good to me, but someone else must approve
  Friendly Automation: Approved for Submit



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 8e1fb37..167a766 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -3429,39 +3429,6 @@
 	ast_rtp_dtls_stop(instance);
 #endif
 
-	/* Destroy the smoother that was smoothing out audio if present */
-	if (rtp->smoother) {
-		ast_smoother_free(rtp->smoother);
-	}
-
-	/* Close our own socket so we no longer get packets */
-	if (rtp->s > -1) {
-		close(rtp->s);
-	}
-
-	/* Destroy RTCP if it was being used */
-	if (rtp->rtcp) {
-		/*
-		 * It is not possible for there to be an active RTCP scheduler
-		 * entry at this point since it holds a reference to the
-		 * RTP instance while it's active.
-		 */
-		if (rtp->rtcp->s > -1 && rtp->s != rtp->rtcp->s) {
-			close(rtp->rtcp->s);
-		}
-		ast_free(rtp->rtcp->local_addr_str);
-		ast_free(rtp->rtcp);
-	}
-
-	/* Destroy RED if it was being used */
-	if (rtp->red) {
-		ao2_unlock(instance);
-		AST_SCHED_DEL(rtp->sched, rtp->red->schedid);
-		ao2_lock(instance);
-		ast_free(rtp->red);
-		rtp->red = NULL;
-	}
-
 #ifdef HAVE_PJPROJECT
 	pj_thread_register_check();
 
@@ -3519,6 +3486,39 @@
 	}
 #endif
 
+	/* Destroy the smoother that was smoothing out audio if present */
+	if (rtp->smoother) {
+		ast_smoother_free(rtp->smoother);
+	}
+
+	/* Close our own socket so we no longer get packets */
+	if (rtp->s > -1) {
+		close(rtp->s);
+	}
+
+	/* Destroy RTCP if it was being used */
+	if (rtp->rtcp) {
+		/*
+		 * It is not possible for there to be an active RTCP scheduler
+		 * entry at this point since it holds a reference to the
+		 * RTP instance while it's active.
+		 */
+		if (rtp->rtcp->s > -1 && rtp->s != rtp->rtcp->s) {
+			close(rtp->rtcp->s);
+		}
+		ast_free(rtp->rtcp->local_addr_str);
+		ast_free(rtp->rtcp);
+	}
+
+	/* Destroy RED if it was being used */
+	if (rtp->red) {
+		ao2_unlock(instance);
+		AST_SCHED_DEL(rtp->sched, rtp->red->schedid);
+		ao2_lock(instance);
+		ast_free(rtp->red);
+		rtp->red = NULL;
+	}
+
 	ao2_cleanup(rtp->lasttxformat);
 	ao2_cleanup(rtp->lastrxformat);
 	ao2_cleanup(rtp->f.subclass.format);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14450
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ie71add549f12b6cdea7e9dbf976d1bd1d2fc0bdc
Gerrit-Change-Number: 14450
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200601/8942a84a/attachment-0001.html>