[Asterisk-code-review] res_rtp_asterisk: Addressing possible rtp range issues (asterisk[18])

[Friendly Automation]

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

Change subject: res_rtp_asterisk: Addressing possible rtp range issues
......................................................................

res_rtp_asterisk: Addressing possible rtp range issues

res/res_rtp_asterisk.c: Adding 1 to rtpstart if it is deteremined
that rtpstart was configured to be an odd value. Also adding a loop
counter to prevent a possible infinite loop when looking for a free
port.

ASTERISK-27406

Change-Id: I90f07deef0716da4a30206e9f849458b2dbe346b
---
M res/res_rtp_asterisk.c
1 file changed, 13 insertions(+), 3 deletions(-)

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



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index b0a7054..5fdcefe 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -3843,7 +3843,7 @@
 
 static int rtp_allocate_transport(struct ast_rtp_instance *instance, struct ast_rtp *rtp)
 {
-	int x, startplace;
+	int x, startplace, i, maxloops;
 
 	rtp->strict_rtp_state = (strictrtp ? STRICT_RTP_CLOSED : STRICT_RTP_OPEN);
 
@@ -3857,11 +3857,14 @@
 	}
 
 	/* Now actually find a free RTP port to use */
-	x = (rtpend == rtpstart) ? rtpstart : (ast_random() % (rtpend - rtpstart)) + rtpstart;
+	x = (ast_random() % (rtpend - rtpstart)) + rtpstart;
 	x = x & ~1;
 	startplace = x;
 
-	for (;;) {
+	/* Protection against infinite loops in the case there is a potential case where the loop is not broken such as an odd
+	   start port sneaking in (even though this condition is checked at load.) */
+	maxloops = rtpend - rtpstart;
+	for (i = 0; i <= maxloops; i++) {
 		ast_sockaddr_set_port(&rtp->bind_address, x);
 		/* Try to bind, this will tell us whether the port is available or not */
 		if (!ast_bind(rtp->s, &rtp->bind_address)) {
@@ -9703,6 +9706,13 @@
 
 	ast_config_destroy(cfg);
 
+	/* Choosing an odd start port casues issues (like a potential infinite loop) and as odd parts are not
+	   chosen anyway, we are going to round up and issue a warning */
+	if (rtpstart & 1) {
+		rtpstart++;
+		ast_log(LOG_WARNING, "Odd start value for RTP port in rtp.conf, rounding up to %d\n", rtpstart);
+	}
+
 	if (rtpstart >= rtpend) {
 		ast_log(LOG_WARNING, "Unreasonable values for RTP start/end port in rtp.conf\n");
 		rtpstart = DEFAULT_RTP_START;

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

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I90f07deef0716da4a30206e9f849458b2dbe346b
Gerrit-Change-Number: 17613
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Bradeen <mbradeen 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/20211206/6635689c/attachment-0001.html>