[Asterisk-code-review] Audit improper usage of scheduler exposed by 5c713fdf18f. (v... (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: Audit improper usage of scheduler exposed by 5c713fdf18f. (v13 additions)
......................................................................


Audit improper usage of scheduler exposed by 5c713fdf18f. (v13 additions)

chan_sip.c:
* Initialize mwi subscription scheduler ids earlier because of ASTOBJ to
ao2 conversion.

* Initialize register scheduler ids earlier because of ASTOBJ to ao2
conversion.

chan_skinny.c:
* Fix more scheduler usage for the valid 0 id value.

ASTERISK-25476

Change-Id: If9f0e5d99638b2f9d102d1ebc9c5a14b2d706e95
---
M channels/chan_sip.c
M channels/chan_skinny.c
2 files changed, 26 insertions(+), 13 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index bc65784..98df83a 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -9374,6 +9374,9 @@
 		return -1;
 	}
 
+	reg->expire = -1;
+	reg->timeout = -1;
+
 	if (ast_string_field_init(reg, 256)) {
 		ao2_t_ref(reg, -1, "failed to string_field_init, drop reg");
 		return -1;
@@ -9447,6 +9450,8 @@
 		return -1;
 	}
 
+	mwi->resub = -1;
+
 	if (ast_string_field_init(mwi, 256)) {
 		ao2_t_ref(mwi, -1, "failed to string_field_init, drop mwi");
 		return -1;
@@ -9461,7 +9466,6 @@
 	}
 	ast_string_field_set(mwi, hostname, hostname);
 	ast_string_field_set(mwi, mailbox, mailbox);
-	mwi->resub = -1;
 	mwi->portno = portnum;
 	mwi->transport = transport;
 
diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index 6ad6a93..5cdfe1b 100644
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -1364,6 +1364,12 @@
 /* How long to wait for an extra digit, if there is an ambiguous match */
 static int matchdigittimeout = 3000;
 
+/*!
+ * To apease the stupid compiler option on ast_sched_del()
+ * since we don't care about the return value.
+ */
+static int not_used;
+
 #define SUBSTATE_UNSET 0
 #define SUBSTATE_OFFHOOK 1
 #define SUBSTATE_ONHOOK 2
@@ -2262,10 +2268,10 @@
 	int instance;
 	int res = -1;
 
-	if (s->auth_timeout_sched && ast_sched_del(sched, s->auth_timeout_sched)) {
-		return 0;
+	if (-1 < s->auth_timeout_sched) {
+		not_used = ast_sched_del(sched, s->auth_timeout_sched);
+		s->auth_timeout_sched = -1;
 	}
-	s->auth_timeout_sched = 0;
 
 	AST_LIST_LOCK(&devices);
 	AST_LIST_TRAVERSE(&devices, d, list){
@@ -5571,6 +5577,7 @@
 			sub->cfwd_sched = -1;
 		} else if (state == SUBSTATE_ONHOOK) {
 			skinny_sched_del(sub->cfwd_sched, sub);
+			sub->cfwd_sched = -1;
 		}
 	}
 
@@ -6170,9 +6177,7 @@
 
 static void handle_keepalive_message(struct skinny_req *req, struct skinnysession *s)
 {
-	if (ast_sched_del(sched, s->keepalive_timeout_sched)) {
-		return;
-	}
+	not_used = ast_sched_del(sched, s->keepalive_timeout_sched);
 
 #ifdef AST_DEVMODE
 	{
@@ -7415,7 +7420,7 @@
 {
 	struct skinnysession *s = (struct skinnysession *)data;
 	ast_log(LOG_WARNING, "Skinny Client failed to authenticate in %d seconds (SCHED %d)\n", auth_timeout, s->auth_timeout_sched);
-	s->auth_timeout_sched = 0;
+	s->auth_timeout_sched = -1;
 	end_session(s);
 	return 0;
 }
@@ -7424,7 +7429,7 @@
 {
 	struct skinnysession *s = (struct skinnysession *)data;
 	ast_log(LOG_WARNING, "Skinny Client failed to send keepalive in last %d seconds (SCHED %d)\n", keep_alive*3, s->keepalive_timeout_sched);
-	s->keepalive_timeout_sched = 0;
+	s->keepalive_timeout_sched = -1;
 	end_session(s);
 	return 0;
 }
@@ -7442,11 +7447,13 @@
 		ast_mutex_unlock(&s->lock);
 	}
 
-	if (s->auth_timeout_sched && !ast_sched_del(sched, s->auth_timeout_sched)) {
-		s->auth_timeout_sched = 0;
+	if (-1 < s->auth_timeout_sched) {
+		not_used = ast_sched_del(sched, s->auth_timeout_sched);
+		s->auth_timeout_sched = -1;
 	}
-	if (s->keepalive_timeout_sched && !ast_sched_del(sched, s->keepalive_timeout_sched)) {
-		s->keepalive_timeout_sched = 0;
+	if (-1 < s->keepalive_timeout_sched) {
+		not_used = ast_sched_del(sched, s->keepalive_timeout_sched);
+		s->keepalive_timeout_sched = -1;
 	}
 
 	if (d) {
@@ -7651,6 +7658,8 @@
 		ast_mutex_init(&s->lock);
 		memcpy(&s->sin, &sin, sizeof(sin));
 		s->fd = as;
+		s->auth_timeout_sched = -1;
+		s->keepalive_timeout_sched = -1;
 
 		if (ast_pthread_create(&s->t, NULL, skinny_session, s)) {
 			destroy_session(s);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: If9f0e5d99638b2f9d102d1ebc9c5a14b2d706e95
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>