[Asterisk-code-review] res pjsip: Fix SEGV on pending-qualify contacts (asterisk[13])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: res_pjsip: Fix SEGV on pending-qualify contacts
......................................................................


res_pjsip: Fix SEGV on pending-qualify contacts

Permanent contacts that hadn't been qualified yet were missing
their contact_status entries causing SEGVs when running CLI
commands.

This patch makes sure that contact_statuses are created for
both dynamic and permanent contacts when they are created.
It also adds checks in the CLI code to make sure there's a
contact_status, just in case.

ASTERISK-25018 #close
Reported-by: Ivan Poddubny
Tested-by: Ivan Poddubny
Tested-by: George Joseph

Change-Id: I3cc13e5cedcafb24c400368b515b02d7fb81e029
---
M res/res_pjsip/include/res_pjsip_private.h
M res/res_pjsip/location.c
M res/res_pjsip/pjsip_options.c
3 files changed, 31 insertions(+), 9 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, approved; Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h
index bf428d5..c1f7e23 100644
--- a/res/res_pjsip/include/res_pjsip_private.h
+++ b/res/res_pjsip/include/res_pjsip_private.h
@@ -313,4 +313,9 @@
  */
 int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
 
+/*!
+ * \internal
+ * \brief Finds or creates contact_status for a contact
+ */
+struct ast_sip_contact_status *ast_res_pjsip_find_or_create_contact_status(const struct ast_sip_contact *contact);
 #endif /* RES_PJSIP_PRIVATE_H_ */
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 45370dd..887053b 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -376,6 +376,11 @@
 			return -1;
 		}
 
+		if (!ast_res_pjsip_find_or_create_contact_status(contact)) {
+			ao2_ref(contact, -1);
+			return -1;
+		}
+
 		ast_string_field_set(contact, uri, contact_uri);
 		ao2_link(aor->permanent_contacts, contact);
 		ao2_ref(contact, -1);
@@ -750,8 +755,8 @@
 		"Contact",
 		flexwidth, flexwidth,
 		wrapper->contact_id,
-		ast_sip_get_contact_short_status_label(status->status),
-		(status->status != UNKNOWN ? ((long long) status->rtt) / 1000.0 : NAN));
+		ast_sip_get_contact_short_status_label(status ? status->status : UNKNOWN),
+		(status && (status->status != UNKNOWN) ? ((long long) status->rtt) / 1000.0 : NAN));
 
 	return 0;
 }
@@ -874,6 +879,17 @@
 struct ast_sip_cli_formatter_entry *contact_formatter;
 struct ast_sip_cli_formatter_entry *aor_formatter;
 
+/*! \brief Always create a contact_status for each contact */
+static int contact_apply_handler(const struct ast_sorcery *sorcery, void *object)
+{
+	struct ast_sip_contact_status *status;
+	struct ast_sip_contact *contact = object;
+
+	status = ast_res_pjsip_find_or_create_contact_status(contact);
+
+	return status ? 0 : -1;
+}
+
 /*! \brief Initialize sorcery with location support */
 int ast_sip_initialize_sorcery_location(void)
 {
@@ -881,7 +897,7 @@
 	ast_sorcery_apply_default(sorcery, "contact", "astdb", "registrar");
 	ast_sorcery_apply_default(sorcery, "aor", "config", "pjsip.conf,criteria=type=aor");
 
-	if (ast_sorcery_object_register(sorcery, "contact", contact_alloc, NULL, NULL) ||
+	if (ast_sorcery_object_register(sorcery, "contact", contact_alloc, NULL, contact_apply_handler) ||
 		ast_sorcery_object_register(sorcery, "aor", aor_alloc, NULL, NULL)) {
 		return -1;
 	}
diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index 40b6f7b..8ffb88c 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -76,11 +76,10 @@
 }
 
 /*!
- * \internal
  * \brief Retrieve a ast_sip_contact_status object from sorcery creating
  *        one if not found.
  */
-static struct ast_sip_contact_status *find_or_create_contact_status(const struct ast_sip_contact *contact)
+struct ast_sip_contact_status *ast_res_pjsip_find_or_create_contact_status(const struct ast_sip_contact *contact)
 {
 	struct ast_sip_contact_status *status;
 
@@ -97,6 +96,10 @@
 			contact->uri);
 		return NULL;
 	}
+
+	status->status = UNKNOWN;
+	status->rtt_start = ast_tv(0, 0);
+	status->rtt = 0;
 
 	if (ast_sorcery_create(ast_sip_get_sorcery(), status)) {
 		ast_log(LOG_ERROR, "Unable to persist ast_sip_contact_status for contact %s\n",
@@ -118,7 +121,7 @@
 	struct ast_sip_contact_status *status;
 	struct ast_sip_contact_status *update;
 
-	status = find_or_create_contact_status(contact);
+	status = ast_res_pjsip_find_or_create_contact_status(contact);
 	if (!status) {
 		ast_log(LOG_ERROR, "Unable to find ast_sip_contact_status for contact %s\n",
 			contact->uri);
@@ -142,8 +145,6 @@
 		ast_tvdiff_us(ast_tvnow(), status->rtt_start) : 0;
 
 	update->rtt_start = ast_tv(0, 0);
-
-
 
 	ast_test_suite_event_notify("AOR_CONTACT_QUALIFY_RESULT",
 		"Contact: %s\r\n"
@@ -172,7 +173,7 @@
 	struct ast_sip_contact_status *status;
 	struct ast_sip_contact_status *update;
 
-	status = find_or_create_contact_status(contact);
+	status = ast_res_pjsip_find_or_create_contact_status(contact);
 	if (!status) {
 		ast_log(LOG_ERROR, "Unable to find ast_sip_contact_status for contact %s\n",
 			contact->uri);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3cc13e5cedcafb24c400368b515b02d7fb81e029
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>