[Asterisk-code-review] Audit improper usage of scheduler exposed by 5c713fdf18f. (asterisk[certified/11.6])

Joshua Colp asteriskteam at digium.com
Thu Dec 3 05:52:02 CST 2015


Joshua Colp has submitted this change and it was merged.

Change subject: Audit improper usage of scheduler exposed by 5c713fdf18f.
......................................................................


Audit improper usage of scheduler exposed by 5c713fdf18f.

channels/chan_iax2.c:
* Initialize struct chan_iax2_pvt scheduler ids earlier because of
iax2_destroy_helper().

channels/chan_sip.c:
channels/sip/config_parser.c:
* Fix initialization of scheduler id struct members.  Some off nominal
paths had 0 as a scheduler id to be destroyed when it was never started.

chan_skinny.c:
* Fix some scheduler id comparisons that excluded the valid 0 id.

channel.c:
* Fix channel initialization of the video stream scheduler id.

pbx_dundi.c:
* Fix channel initialization of the packet retransmission scheduler id.

ASTERISK-25476

Change-Id: I07a3449f728f671d326a22fcbd071f150ba2e8c8
---
M channels/chan_iax2.c
M channels/chan_sip.c
M channels/chan_skinny.c
M channels/sip/config_parser.c
M main/channel.c
M pbx/pbx_dundi.c
6 files changed, 29 insertions(+), 17 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index a492bdd..3943d72 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -2139,25 +2139,26 @@
 		return NULL;
 	}
 
-	if (ast_string_field_init(tmp, 32)) {
-		ao2_ref(tmp, -1);
-		tmp = NULL;
-		return NULL;
-	}
-		
-	tmp->prefs = prefs;
 	tmp->pingid = -1;
 	tmp->lagid = -1;
 	tmp->autoid = -1;
 	tmp->authid = -1;
 	tmp->initid = -1;
 	tmp->keyrotateid = -1;
+	tmp->jbid = -1;
+
+	if (ast_string_field_init(tmp, 32)) {
+		ao2_ref(tmp, -1);
+		tmp = NULL;
+		return NULL;
+	}
+
+	tmp->prefs = prefs;
 
 	ast_string_field_set(tmp,exten, "s");
 	ast_string_field_set(tmp,host, host);
 
 	tmp->jb = jb_new();
-	tmp->jbid = -1;
 	jbconf.max_jitterbuf = maxjitterbuffer;
 	jbconf.resync_threshold = resyncthreshold;
 	jbconf.max_contig_interp = maxjitterinterps;
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index c47baf7..b652cb2 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -1083,6 +1083,7 @@
 	ast_assert(esc != NULL);
 
 	ao2_unlink(esc->compositor, esc_entry);
+	esc_entry->sched_id = -1;
 	ao2_ref(esc_entry, -1);
 	return 0;
 }
@@ -1115,6 +1116,11 @@
 	/* Bump refcount for scheduler */
 	ao2_ref(esc_entry, +1);
 	esc_entry->sched_id = ast_sched_add(sched, expires_ms, publish_expire, esc_entry);
+	if (esc_entry->sched_id == -1) {
+		ao2_ref(esc_entry, -1);
+		ao2_ref(esc_entry, -1);
+		return NULL;
+	}
 
 	/* Note: This links the esc_entry into the ESC properly */
 	create_new_sip_etag(esc_entry, 0);
diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index c64d8de..d9b2577 100644
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -4374,7 +4374,7 @@
 
 	AST_LIST_TRAVERSE(ast_channel_varshead(ast), current, entries) {
 		if (!(strcasecmp(ast_var_name(current),"SKINNY_AUTOANSWER"))) {
-			if (d->hookstate == SKINNY_ONHOOK && !sub->aa_sched < 0) {
+			if (d->hookstate == SKINNY_ONHOOK && sub->aa_sched < 0) {
 				char buf[24];
 				int aatime;
 				char *stringp = buf, *curstr;
@@ -4976,12 +4976,12 @@
 		return;
 	}
 
-	if (sub->dialer_sched) {
+	if (-1 < sub->dialer_sched) {
 		skinny_sched_del(sub->dialer_sched, sub);
 		sub->dialer_sched = -1;
 	}
 
-	if (state != SUBSTATE_RINGIN && sub->aa_sched) {
+	if (state != SUBSTATE_RINGIN && -1 < sub->aa_sched) {
 		skinny_sched_del(sub->aa_sched, sub);
 		sub->aa_sched = -1;
 		sub->aa_beep = 0;
@@ -5635,7 +5635,7 @@
 	}
 
 	if ((sub->owner && ast_channel_state(sub->owner) <  AST_STATE_UP)) {
-		if (sub->dialer_sched &&	!skinny_sched_del(sub->dialer_sched, sub)) {
+		if (-1 < sub->dialer_sched && !skinny_sched_del(sub->dialer_sched, sub)) {
 			SKINNY_DEBUG(DEBUG_SUB, 3, "Sub %d - Got a digit and not timed out, so try dialing\n", sub->callid);
 			sub->dialer_sched = -1;
 			len = strlen(sub->exten);
@@ -6519,7 +6519,7 @@
 	case SOFTKEY_BKSPC:
 		SKINNY_DEBUG(DEBUG_PACKET, 3, "Received SOFTKEY_BKSPC from %s, inst %d, callref %d\n",
 			d->name, instance, callreference);
-		if (sub->dialer_sched && !skinny_sched_del(sub->dialer_sched, sub)) {
+		if (-1 < sub->dialer_sched && !skinny_sched_del(sub->dialer_sched, sub)) {
 			size_t len;
 			sub->dialer_sched = -1;
 			len = strlen(sub->exten);
diff --git a/channels/sip/config_parser.c b/channels/sip/config_parser.c
index fe34283..b2c91d1 100644
--- a/channels/sip/config_parser.c
+++ b/channels/sip/config_parser.c
@@ -79,13 +79,17 @@
 		AST_APP_ARG(port);
 	);
 
+	if (!reg) {
+		return -1;
+	}
+
+	reg->expire = -1;
+	reg->timeout = -1;
+
 	if (!value) {
 		return -1;
 	}
 
-	if (!reg) {
-		return -1;
-	}
 	ast_copy_string(buf, value, sizeof(buf));
 
 	/*! register => [peer?][transport://]user[@domain][:secret[:authuser]]@host[:port][/extension][~expiry]
@@ -261,7 +265,6 @@
 	ast_string_field_set(reg, regdomain, ast_strip_quoted(S_OR(user2.domain, ""), "\"", "\""));
 
 	reg->transport = transport;
-	reg->timeout = reg->expire = -1;
 	reg->portno = portnum;
 	reg->regdomainport = domainport;
 	reg->callid_valid = FALSE;
diff --git a/main/channel.c b/main/channel.c
index b623fd5..09fbe5d 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1049,6 +1049,7 @@
 	ast_channel_state_set(tmp, state);
 
 	ast_channel_streamid_set(tmp, -1);
+	ast_channel_vstreamid_set(tmp, -1);
 
 	ast_channel_fin_set(tmp, global_fin);
 	ast_channel_fout_set(tmp, global_fout);
diff --git a/pbx/pbx_dundi.c b/pbx/pbx_dundi.c
index ba1e8e5..83ffb8f 100644
--- a/pbx/pbx_dundi.c
+++ b/pbx/pbx_dundi.c
@@ -3256,6 +3256,7 @@
 	pack = ast_calloc(1, len);
 	if (pack) {
 		pack->h = (struct dundi_hdr *)(pack->data);
+		pack->retransid = -1;
 		if (cmdresp != DUNDI_COMMAND_ACK) {
 			pack->retransid = ast_sched_add(sched, trans->retranstimer, dundi_rexmit, pack);
 			pack->retrans = DUNDI_DEFAULT_RETRANS - 1;

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I07a3449f728f671d326a22fcbd071f150ba2e8c8
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: certified/11.6
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>



More information about the asterisk-code-review mailing list