[Asterisk-code-review] pjsip_transport_events: Fix possible use after free on transport (asterisk[18])

Friendly Automation asteriskteam at digium.com
Sat Dec 3 10:24:35 CST 2022


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

Change subject: pjsip_transport_events: Fix possible use after free on transport
......................................................................

pjsip_transport_events: Fix possible use after free on transport

It was possible for a module that registered for transport monitor
events to pass in a pjsip_transport that had already been freed.
This caused pjsip_transport_events to crash when looking up the
monitor for the transport.  The fix is a two pronged approach.

1. We now increment the reference count on pjsip_transports when we
create monitors for them, then decrement the count when the
transport is going to be destroyed.

2. There are now APIs to register and unregister monitor callbacks
by "transport key" which is a string concatenation of the remote ip
address and port.  This way the module needing to monitor the
transport doesn't have to hold on to the transport object itself to
unregister.  It just has to save the transport_key.

* Added the pjsip_transport reference increment and decrement.

* Changed the internal transport monitor container key from the
  transport->obj_name (which may not be unique anyway) to the
  transport_key.

* Added a helper macro AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR() that
  fills a buffer with the transport_key using a passed-in
  pjsip_transport.

* Added the following functions:
  ast_sip_transport_monitor_register_key
  ast_sip_transport_monitor_register_replace_key
  ast_sip_transport_monitor_unregister_key
  and marked their non-key counterparts as deprecated.

* Updated res_pjsip_pubsub and res_pjsip_outbound_register to use
  the new "key" monitor functions.

NOTE: res_pjsip_registrar also uses the transport monitor
functionality but doesn't have a persistent object other than
contact to store a transport key.  At this time, it continues to
use the non-key monitor functions.

ASTERISK-30244

Change-Id: I1a20baf2a8643c272dcf819871d6c395f148f00b
---
M include/asterisk/res_pjsip.h
M res/res_pjsip/pjsip_transport_events.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_pubsub.c
4 files changed, 357 insertions(+), 43 deletions(-)

Approvals:
  Friendly Automation: Looks good to me, approved; Approved for Submit




diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index d207297..f31cd10 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -87,6 +87,26 @@
 #define AST_STIR_SHAKEN_RESPONSE_STR_UNSUPPORTED_CREDENTIAL "Unsupported Credential"
 #define AST_STIR_SHAKEN_RESPONSE_STR_INVALID_IDENTITY_HEADER "Invalid Identity Header"
 
+/* ":12345" */
+#define COLON_PORT_STRLEN 6
+/*
+ * "<ipaddr>:<port>"
+ * PJ_INET6_ADDRSTRLEN includes the NULL terminator
+ */
+#define IP6ADDR_COLON_PORT_BUFLEN (PJ_INET6_ADDRSTRLEN + COLON_PORT_STRLEN)
+
+/*!
+ * \brief Fill a buffer with a pjsip transport's remote ip address and port
+ *
+ * \param transport The pjsip_transport to use
+ * \param dest The destination buffer of at least IP6ADDR_COLON_PORT_BUFLEN bytes
+ */
+#define AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(_transport, _dest) \
+	snprintf(_dest, IP6ADDR_COLON_PORT_BUFLEN, \
+		PJSTR_PRINTF_SPEC ":%d", \
+		PJSTR_PRINTF_VAR(_transport->remote_name.host), \
+		_transport->remote_name.port);
+
 /* Forward declarations of PJSIP stuff */
 struct pjsip_rx_data;
 struct pjsip_module;
@@ -3745,6 +3765,7 @@
 
 /*!
  * \brief Register a reliable transport shutdown monitor callback.
+ * \deprecated Replaced with ast_sip_transport_monitor_register_key().
  * \since 13.20.0
  *
  * \param transport Transport to monitor for shutdown.
@@ -3763,7 +3784,28 @@
 	ast_transport_monitor_shutdown_cb cb, void *ao2_data);
 
 /*!
+ * \brief Register a reliable transport shutdown monitor callback.
+ *
+ * \param transport_key Key for the transport to monitor for shutdown.
+ *                      Create the key with AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR.
+ * \param cb Who to call when transport is shutdown.
+ * \param ao2_data Data to pass with the callback.
+ *
+ * \note The data object passed will have its reference count automatically
+ * incremented by this call and automatically decremented after the callback
+ * runs or when the callback is unregistered.
+ *
+ * There is no checking for duplicate registrations.
+ *
+ * \return enum ast_transport_monitor_reg
+ */
+enum ast_transport_monitor_reg ast_sip_transport_monitor_register_key(
+	const char *transport_key, ast_transport_monitor_shutdown_cb cb,
+	void *ao2_data);
+
+/*!
  * \brief Register a reliable transport shutdown monitor callback replacing any duplicate.
+ * \deprecated Replaced with ast_sip_transport_monitor_register_replace_key().
  * \since 13.26.0
  * \since 16.3.0
  *
@@ -3786,7 +3828,31 @@
 	ast_transport_monitor_shutdown_cb cb, void *ao2_data, ast_transport_monitor_data_matcher matches);
 
 /*!
+ * \brief Register a reliable transport shutdown monitor callback replacing any duplicate.
+ *
+ * \param transport_key Key for the transport to monitor for shutdown.
+ *                      Create the key with AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR.
+ * \param cb Who to call when transport is shutdown.
+ * \param ao2_data Data to pass with the callback.
+ * \param matches Matcher function that returns true if data matches a previously
+ *                registered data object
+ *
+ * \note The data object passed will have its reference count automatically
+ * incremented by this call and automatically decremented after the callback
+ * runs or when the callback is unregistered.
+ *
+ * This function checks for duplicates, and overwrites/replaces the old monitor
+ * with the given one.
+ *
+ * \return enum ast_transport_monitor_reg
+ */
+enum ast_transport_monitor_reg ast_sip_transport_monitor_register_replace_key(
+	const char *transport_key, ast_transport_monitor_shutdown_cb cb,
+	void *ao2_data, ast_transport_monitor_data_matcher matches);
+
+/*!
  * \brief Unregister a reliable transport shutdown monitor
+ * \deprecated Replaced with ast_sip_transport_monitor_unregister_key().
  * \since 13.20.0
  *
  * \param transport Transport to monitor for shutdown.
@@ -3803,6 +3869,23 @@
 	ast_transport_monitor_shutdown_cb cb, void *data, ast_transport_monitor_data_matcher matches);
 
 /*!
+ * \brief Unregister a reliable transport shutdown monitor
+ *
+ * \param transport_key Key for the transport to monitor for shutdown.
+ *                      Create the key with AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR.
+ * \param cb The callback that was used for the original register.
+ * \param data Data to pass to the matcher. May be NULL and does NOT need to be an ao2 object.
+ *             If NULL, all monitors with the provided callback are unregistered.
+ * \param matches Matcher function that returns true if data matches the previously
+ *                registered data object.  If NULL, a simple pointer comparison is done.
+ *
+ * \note The data object passed into the original register will have its reference count
+ * automatically decremented.
+ */
+void ast_sip_transport_monitor_unregister_key(const char *transport_key,
+	ast_transport_monitor_shutdown_cb cb, void *data, ast_transport_monitor_data_matcher matches);
+
+/*!
  * \brief Unregister a transport shutdown monitor from all reliable transports
  * \since 13.20.0
  *
diff --git a/res/res_pjsip/pjsip_transport_events.c b/res/res_pjsip/pjsip_transport_events.c
index a816f46..0e4705d 100644
--- a/res/res_pjsip/pjsip_transport_events.c
+++ b/res/res_pjsip/pjsip_transport_events.c
@@ -30,6 +30,7 @@
 #include "asterisk.h"
 
 #include "asterisk/res_pjsip.h"
+#include "asterisk/res_pjsip_cli.h"
 #include "include/res_pjsip_private.h"
 #include "asterisk/linkedlists.h"
 #include "asterisk/vector.h"
@@ -49,8 +50,14 @@
 
 /*! \brief Structure for transport to be monitored */
 struct transport_monitor {
+	/*! \brief Key <ipaddr>:<port> */
+	char key[IP6ADDR_COLON_PORT_BUFLEN];
 	/*! \brief The underlying PJSIP transport */
 	pjsip_transport *transport;
+	/*! For debugging purposes, we save the obj_name
+	 * in case the transport goes away.
+	 */
+	char *transport_obj_name;
 	/*! Who is interested in when this transport shuts down. */
 	AST_VECTOR(, struct transport_monitor_notifier) monitors;
 };
@@ -64,12 +71,14 @@
 /*! List of registered transport state callbacks. */
 static AST_RWLIST_HEAD(, ast_sip_tpmgr_state_callback) transport_state_list;
 
-
 /*! \brief Hashing function for struct transport_monitor */
-AO2_STRING_FIELD_HASH_FN(transport_monitor, transport->obj_name);
+AO2_STRING_FIELD_HASH_FN(transport_monitor, key);
 
 /*! \brief Comparison function for struct transport_monitor */
-AO2_STRING_FIELD_CMP_FN(transport_monitor, transport->obj_name);
+AO2_STRING_FIELD_CMP_FN(transport_monitor, key);
+
+/*! \brief Sort function for struct transport_monitor */
+AO2_STRING_FIELD_SORT_FN(transport_monitor, key);
 
 static const char *transport_state2str(pjsip_transport_state state)
 {
@@ -112,6 +121,11 @@
 		ao2_cleanup(notifier->data);
 	}
 	AST_VECTOR_FREE(&monitored->monitors);
+	ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : state:MONITOR_DESTROYED\n",
+		monitored->key, monitored->transport->obj_name,
+		monitored->transport->type_name,pj_atomic_get(monitored->transport->ref_cnt));
+	ast_free(monitored->transport_obj_name);
+	pjsip_transport_dec_ref(monitored->transport);
 }
 
 /*!
@@ -125,8 +139,11 @@
 static void transport_state_do_reg_callbacks(struct ao2_container *transports, pjsip_transport *transport)
 {
 	struct transport_monitor *monitored;
+	char key[IP6ADDR_COLON_PORT_BUFLEN];
 
-	monitored = ao2_find(transports, transport->obj_name, OBJ_SEARCH_KEY | OBJ_UNLINK);
+	AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(transport, key);
+
+	monitored = ao2_find(transports, key, OBJ_SEARCH_KEY | OBJ_UNLINK);
 	if (monitored) {
 		int idx;
 
@@ -134,8 +151,10 @@
 			struct transport_monitor_notifier *notifier;
 
 			notifier = AST_VECTOR_GET_ADDR(&monitored->monitors, idx);
-			ast_debug(3, "running callback %p(%p) for transport %s\n",
-				notifier->cb, notifier->data, transport->obj_name);
+			ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : running callback %p(%p)\n",
+				monitored->key, monitored->transport->obj_name,
+				monitored->transport->type_name,
+				pj_atomic_get(monitored->transport->ref_cnt), notifier->cb, notifier->data);
 			notifier->cb(notifier->data);
 		}
 		ao2_ref(monitored, -1);
@@ -269,8 +288,11 @@
 		&& (transports = ao2_global_obj_ref(active_transports))) {
 		struct transport_monitor *monitored;
 
-		ast_debug(3, "Reliable transport '%s' state:%s\n",
-			transport->obj_name, transport_state2str(state));
+		ast_debug(3, "Transport " PJSTR_PRINTF_SPEC ":%d(%s,%s): RefCnt: %ld state:%s\n",
+			PJSTR_PRINTF_VAR(transport->remote_name.host),
+			transport->remote_name.port, transport->obj_name,
+			transport->type_name,
+			pj_atomic_get(transport->ref_cnt), transport_state2str(state));
 		switch (state) {
 		case PJSIP_TP_STATE_CONNECTED:
 			if (PJSIP_TRANSPORT_IS_SECURE(transport) &&
@@ -285,10 +307,18 @@
 				break;
 			}
 			monitored->transport = transport;
+			AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(transport, monitored->key);
+			monitored->transport_obj_name = ast_strdup(transport->obj_name);
+
 			if (AST_VECTOR_INIT(&monitored->monitors, 5)) {
 				ao2_ref(monitored, -1);
 				break;
 			}
+			pjsip_transport_add_ref(monitored->transport);
+			ast_debug(3, "Transport %s(%s,%s): RefCnt: %ld state:MONITOR_CREATED\n",
+				monitored->key,	monitored->transport_obj_name,
+				monitored->transport->type_name,
+				pj_atomic_get(monitored->transport->ref_cnt));
 
 			ao2_link(transports, monitored);
 			ao2_ref(monitored, -1);
@@ -362,8 +392,10 @@
 			|| cb_data->matches(cb_data->data, notifier->data))) {
 			ao2_cleanup(notifier->data);
 			AST_VECTOR_REMOVE_UNORDERED(&monitored->monitors, idx);
-			ast_debug(3, "Unregistered monitor %p(%p) from transport %s\n",
-				notifier->cb, notifier->data, monitored->transport->obj_name);
+			ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : Unregistered monitor %p(%p)\n",
+				monitored->key, monitored->transport_obj_name,
+				monitored->transport->type_name,
+				pj_atomic_get(monitored->transport->ref_cnt), notifier->cb, notifier->data);
 		}
 	}
 	return 0;
@@ -397,10 +429,18 @@
 void ast_sip_transport_monitor_unregister(pjsip_transport *transport,
 	ast_transport_monitor_shutdown_cb cb, void *data, ast_transport_monitor_data_matcher matches)
 {
+	char key[IP6ADDR_COLON_PORT_BUFLEN];
+	AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(transport, key);
+	ast_sip_transport_monitor_unregister_key(key, cb, data, matches);
+}
+
+void ast_sip_transport_monitor_unregister_key(const char *transport_key,
+	ast_transport_monitor_shutdown_cb cb, void *data, ast_transport_monitor_data_matcher matches)
+{
 	struct ao2_container *transports;
 	struct transport_monitor *monitored;
 
-	ast_assert(transport != NULL && cb != NULL);
+	ast_assert(transport_key != NULL && cb != NULL);
 
 	transports = ao2_global_obj_ref(active_transports);
 	if (!transports) {
@@ -408,7 +448,7 @@
 	}
 
 	ao2_lock(transports);
-	monitored = ao2_find(transports, transport->obj_name, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+	monitored = ao2_find(transports, transport_key, OBJ_SEARCH_KEY | OBJ_NOLOCK);
 	if (monitored) {
 		struct callback_data cb_data = {
 			.cb = cb,
@@ -426,17 +466,35 @@
 enum ast_transport_monitor_reg ast_sip_transport_monitor_register(pjsip_transport *transport,
 	ast_transport_monitor_shutdown_cb cb, void *ao2_data)
 {
-	return ast_sip_transport_monitor_register_replace(transport, cb, ao2_data, NULL);
+	char key[IP6ADDR_COLON_PORT_BUFLEN];
+	AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(transport, key);
+
+	return ast_sip_transport_monitor_register_replace_key(key, cb, ao2_data, NULL);
+}
+
+enum ast_transport_monitor_reg ast_sip_transport_monitor_register_key(const char *transport_key,
+	ast_transport_monitor_shutdown_cb cb, void *ao2_data)
+{
+	return ast_sip_transport_monitor_register_replace_key(transport_key, cb, ao2_data, NULL);
 }
 
 enum ast_transport_monitor_reg ast_sip_transport_monitor_register_replace(pjsip_transport *transport,
 	ast_transport_monitor_shutdown_cb cb, void *ao2_data, ast_transport_monitor_data_matcher matches)
 {
+	char key[IP6ADDR_COLON_PORT_BUFLEN];
+
+	AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(transport, key);
+	return ast_sip_transport_monitor_register_replace_key(key, cb, ao2_data, NULL);
+}
+
+enum ast_transport_monitor_reg ast_sip_transport_monitor_register_replace_key(const char *transport_key,
+	ast_transport_monitor_shutdown_cb cb, void *ao2_data, ast_transport_monitor_data_matcher matches)
+{
 	struct ao2_container *transports;
 	struct transport_monitor *monitored;
 	enum ast_transport_monitor_reg res = AST_TRANSPORT_MONITOR_REG_NOT_FOUND;
 
-	ast_assert(transport != NULL && cb != NULL);
+	ast_assert(transport_key != NULL && cb != NULL);
 
 	transports = ao2_global_obj_ref(active_transports);
 	if (!transports) {
@@ -444,7 +502,7 @@
 	}
 
 	ao2_lock(transports);
-	monitored = ao2_find(transports, transport->obj_name, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+	monitored = ao2_find(transports, transport_key, OBJ_SEARCH_KEY | OBJ_NOLOCK);
 	if (monitored) {
 		struct transport_monitor_notifier new_monitor;
 		struct callback_data cb_data = {
@@ -461,12 +519,15 @@
 		if (AST_VECTOR_APPEND(&monitored->monitors, new_monitor)) {
 			ao2_cleanup(ao2_data);
 			res = AST_TRANSPORT_MONITOR_REG_FAILED;
-			ast_debug(3, "Register monitor %p(%p) to transport %s FAILED\n",
-				cb, ao2_data, transport->obj_name);
+			ast_debug(3, "Transport %s(%s) RefCnt: %ld : Monitor registration failed %p(%p)\n",
+				monitored->key, monitored->transport_obj_name,
+				pj_atomic_get(monitored->transport->ref_cnt), cb, ao2_data);
 		} else {
 			res = AST_TRANSPORT_MONITOR_REG_SUCCESS;
-			ast_debug(3, "Registered monitor %p(%p) to transport %s\n",
-				cb, ao2_data, transport->obj_name);
+			ast_debug(3, "Transport %s(%s,%s) RefCnt: %ld : Registered monitor %p(%p)\n",
+				monitored->key, monitored->transport_obj_name,
+				monitored->transport->type_name,
+				pj_atomic_get(monitored->transport->ref_cnt), cb, ao2_data);
 		}
 
 		ao2_ref(monitored, -1);
@@ -499,10 +560,120 @@
 	AST_RWLIST_UNLOCK(&transport_state_list);
 }
 
+static char *cli_show_monitors(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
+{
+	char *cli_rc = CLI_FAILURE;
+	int rc = 0;
+	int using_regex = 0;
+	regex_t regex = { 0, };
+	int container_count;
+	struct ao2_iterator iter;
+	struct ao2_container *sorted_monitors = NULL;
+	struct ao2_container *transports;
+	struct transport_monitor *monitored;
+
+	switch (cmd) {
+	case CLI_INIT:
+		e->command = "pjsip show transport-monitors";
+		e->usage = "Usage: pjsip show transport-monitors [ like <pattern> ]\n"
+		            "      Show pjsip transport monitors\n";
+		return NULL;
+	case CLI_GENERATE:
+		return NULL;
+	}
+
+	if (a->argc != 3 && a->argc != 5) {
+		return CLI_SHOWUSAGE;
+	}
+
+	if (a->argc == 5) {
+		int regrc;
+		if (strcasecmp(a->argv[3], "like")) {
+			return CLI_SHOWUSAGE;
+		}
+		regrc = regcomp(&regex, a->argv[4], REG_EXTENDED | REG_ICASE | REG_NOSUB);
+		if (regrc) {
+			char err[256];
+			regerror(regrc, &regex, err, 256);
+			ast_cli(a->fd, "PJSIP Transport Monitor: Error: %s\n", err);
+			return CLI_FAILURE;
+		}
+		using_regex = 1;
+	}
+
+	/* Get a sorted snapshot of the scheduled tasks */
+	sorted_monitors = ao2_container_alloc_rbtree(AO2_ALLOC_OPT_LOCK_NOLOCK, 0,
+		transport_monitor_sort_fn, NULL);
+	if (!sorted_monitors) {
+		ast_cli(a->fd, "PJSIP Transport Monitor: Unable to allocate temporary container\n");
+		goto error;
+	}
+
+	transports = ao2_global_obj_ref(active_transports);
+	if (!transports) {
+		ast_cli(a->fd, "PJSIP Transport Monitor: Unable to get transports\n");
+		goto error;
+	}
+
+	ao2_lock(transports);
+	rc = ao2_container_dup(sorted_monitors, transports, 0);
+	ao2_unlock(transports);
+	ao2_ref(transports, -1);
+	if (rc != 0) {
+		ast_cli(a->fd, "PJSIP Transport Monitors: Unable to sort temporary container\n");
+		goto error;
+	}
+	container_count = ao2_container_count(sorted_monitors);
+
+	ast_cli(a->fd, "PJSIP Transport Monitors:\n\n");
+
+	ast_cli(a->fd,
+		"<Remote Host...................................> <State.....> <Direction> <RefCnt> <Monitors> <ObjName............>\n");
+
+	iter = ao2_iterator_init(sorted_monitors, AO2_ITERATOR_UNLINK);
+	for (; (monitored = ao2_iterator_next(&iter)); ao2_ref(monitored, -1)) {
+		char *state;
+
+		if (using_regex && regexec(&regex, monitored->key, 0, NULL, 0) == REG_NOMATCH) {
+			continue;
+		}
+
+		if (monitored->transport->is_destroying) {
+			state = "DESTROYING";
+		} else if (monitored->transport->is_shutdown) {
+			state = "SHUTDOWN";
+		} else {
+			state = "ACTIVE";
+		}
+
+		ast_cli(a->fd, " %-46.46s   %-10s   %-9s   %6ld   %8" PRIu64 "   %s\n",
+			monitored->key, state,
+			monitored->transport->dir == PJSIP_TP_DIR_OUTGOING ? "Outgoing" : "Incoming",
+			pj_atomic_get(monitored->transport->ref_cnt),
+			AST_VECTOR_SIZE(&monitored->monitors), monitored->transport->obj_name);
+	}
+	ao2_iterator_destroy(&iter);
+	ast_cli(a->fd, "\nTotal Transport Monitors: %d\n\n", container_count);
+	cli_rc = CLI_SUCCESS;
+error:
+	if (using_regex) {
+		regfree(&regex);
+	}
+	ao2_cleanup(sorted_monitors);
+
+	return cli_rc;
+}
+
+static struct ast_cli_entry cli_commands[] = {
+	AST_CLI_DEFINE(cli_show_monitors, "Show pjsip transport monitors"),
+};
+
 void ast_sip_destroy_transport_events(void)
 {
 	pjsip_tpmgr *tpmgr;
 
+	ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
+
 	tpmgr = pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint());
 	if (tpmgr) {
 		pjsip_tpmgr_set_state_cb(tpmgr, tpmgr_state_callback);
@@ -522,7 +693,7 @@
 	}
 
 	transports = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0,
-		ACTIVE_TRANSPORTS_BUCKETS, transport_monitor_hash_fn, NULL,
+		ACTIVE_TRANSPORTS_BUCKETS, transport_monitor_hash_fn, transport_monitor_sort_fn,
 		transport_monitor_cmp_fn);
 	if (!transports) {
 		return -1;
@@ -533,5 +704,8 @@
 	tpmgr_state_callback = pjsip_tpmgr_get_state_cb(tpmgr);
 	pjsip_tpmgr_set_state_cb(tpmgr, &transport_state_callback);
 
+	ast_cli_register_multiple(cli_commands, ARRAY_LEN(cli_commands));
+
+
 	return 0;
 }
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 79cba6a..fc1dcad 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -993,6 +993,8 @@
 	pjsip_rx_data *rdata;
 	/*! \brief Request for which the response was received */
 	pjsip_tx_data *old_request;
+	/*! \brief Key for the reliable transport in use */
+	char transport_key[IP6ADDR_COLON_PORT_BUFLEN];
 };
 
 /*! \brief Registration response structure destructor */
@@ -1108,13 +1110,10 @@
 	return strcmp(ma, mb) == 0;
 }
 
-static void registration_transport_monitor_setup(pjsip_transport *transport, const char *registration_name)
+static void registration_transport_monitor_setup(const char *transport_key, const char *registration_name)
 {
 	char *monitor;
 
-	if (!PJSIP_TRANSPORT_IS_RELIABLE(transport)) {
-		return;
-	}
 	monitor = ao2_alloc_options(strlen(registration_name) + 1, NULL,
 		AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!monitor) {
@@ -1127,8 +1126,8 @@
 	 * register the monitor.  We might get into a message spamming infinite
 	 * loop of registration, shutdown, reregistration...
 	 */
-	ast_sip_transport_monitor_register(transport, registration_transport_shutdown_cb,
-		monitor);
+	ast_sip_transport_monitor_register_replace_key(transport_key, registration_transport_shutdown_cb,
+		monitor, monitor_matcher);
 	ao2_ref(monitor, -1);
 }
 
@@ -1322,14 +1321,18 @@
 			schedule_registration(response->client_state, next_registration_round);
 
 			/* See if we should monitor for transport shutdown */
-			registration_transport_monitor_setup(response->rdata->tp_info.transport,
-				response->client_state->registration_name);
+			if (PJSIP_TRANSPORT_IS_RELIABLE(response->rdata->tp_info.transport)) {
+				registration_transport_monitor_setup(response->transport_key,
+					response->client_state->registration_name);
+			}
 		} else {
 			ast_debug(1, "Outbound unregistration to '%s' with client '%s' successful\n", server_uri, client_uri);
 			update_client_state_status(response->client_state, SIP_REGISTRATION_UNREGISTERED);
-			ast_sip_transport_monitor_unregister(response->rdata->tp_info.transport,
-				registration_transport_shutdown_cb, response->client_state->registration_name,
-				monitor_matcher);
+			if (PJSIP_TRANSPORT_IS_RELIABLE(response->rdata->tp_info.transport)) {
+				ast_sip_transport_monitor_unregister_key(response->transport_key,
+					registration_transport_shutdown_cb, response->client_state->registration_name,
+					monitor_matcher);
+			}
 		}
 
 		save_response_fields_to_transport(response);
@@ -1445,6 +1448,9 @@
 		response->old_request = tsx->last_tx;
 		pjsip_tx_data_add_ref(response->old_request);
 		pjsip_rx_data_clone(param->rdata, 0, &response->rdata);
+		AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(param->rdata->tp_info.transport,
+			response->transport_key);
+
 	} else {
 		/* old_request steals the reference */
 		response->old_request = client_state->last_tdata;
diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index d767269..26b86cc 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -389,8 +389,8 @@
 	char src_name[PJ_INET6_ADDRSTRLEN];
 	/*! Source port of the message */
 	int src_port;
-	/*! Local transport key type */
-	char transport_key[32];
+	/*! Local transport type (UDP,TCP,TLS)*/
+	char transport_type[32];
 	/*! Local transport address */
 	char local_name[PJ_INET6_ADDRSTRLEN];
 	/*! Local transport port */
@@ -474,7 +474,7 @@
 	/*! The transport the subscription was received on.
 	 * Only used for reliable transports.
 	 */
-	pjsip_transport *transport;
+	char transport_key[IP6ADDR_COLON_PORT_BUFLEN];
 	/*! Indicator if initial notify should be generated.
 	 * Used to refresh modified RLS.
 	 */
@@ -711,8 +711,9 @@
 							rdata->tp_info.transport->obj_name,
 							sub_tree->persistence->endpoint, sub_tree->root->resource,
 							sub_tree->persistence->prune_on_boot);
-						sub_tree->transport = rdata->tp_info.transport;
-						ast_sip_transport_monitor_register(rdata->tp_info.transport,
+						AST_SIP_MAKE_REMOTE_IPADDR_PORT_STR(rdata->tp_info.transport,
+							sub_tree->transport_key);
+						ast_sip_transport_monitor_register_key(sub_tree->transport_key,
 							sub_tree_transport_cb, sub_tree);
 						/*
 						 * FYI: ast_sip_transport_monitor_register holds a reference to the sub_tree
@@ -746,8 +747,8 @@
 		ast_copy_string(sub_tree->persistence->src_name, rdata->pkt_info.src_name,
 				sizeof(sub_tree->persistence->src_name));
 		sub_tree->persistence->src_port = rdata->pkt_info.src_port;
-		ast_copy_string(sub_tree->persistence->transport_key, rdata->tp_info.transport->type_name,
-			sizeof(sub_tree->persistence->transport_key));
+		ast_copy_string(sub_tree->persistence->transport_type, rdata->tp_info.transport->type_name,
+			sizeof(sub_tree->persistence->transport_type));
 		ast_copy_pj_str(sub_tree->persistence->local_name, &rdata->tp_info.transport->local_name.host,
 			sizeof(sub_tree->persistence->local_name));
 		sub_tree->persistence->local_port = rdata->tp_info.transport->local_name.port;
@@ -763,12 +764,12 @@
 		return;
 	}
 
-	if (sub_tree->persistence->prune_on_boot && sub_tree->transport) {
+	if (sub_tree->persistence->prune_on_boot && !ast_strlen_zero(sub_tree->transport_key)) {
 		ast_debug(3, "Unregistering transport monitor on %s '%s->%s'\n",
-			sub_tree->transport->obj_name,
+			sub_tree->transport_key,
 			sub_tree->endpoint ? ast_sorcery_object_get_id(sub_tree->endpoint) : "Unknown",
 			sub_tree->root ? sub_tree->root->resource : "Unknown");
-		ast_sip_transport_monitor_unregister(sub_tree->transport,
+		ast_sip_transport_monitor_unregister_key(sub_tree->transport_key,
 			sub_tree_transport_cb, sub_tree, NULL);
 	}
 
@@ -1743,7 +1744,7 @@
 	rdata.tp_info.pool = pool;
 
 	if (ast_sip_create_rdata_with_contact(&rdata, persistence->packet, persistence->src_name,
-		persistence->src_port, persistence->transport_key, persistence->local_name,
+		persistence->src_port, persistence->transport_type, persistence->local_name,
 		persistence->local_port, persistence->contact_uri)) {
 		ast_log(LOG_WARNING, "Failed recreating '%s' subscription: The message could not be parsed\n",
 			persistence->endpoint);
@@ -5759,7 +5760,7 @@
 	ast_sorcery_object_field_register(sorcery, "subscription_persistence", "src_port", "0", OPT_UINT_T, 0,
 		FLDSET(struct subscription_persistence, src_port));
 	ast_sorcery_object_field_register(sorcery, "subscription_persistence", "transport_key", "0", OPT_CHAR_ARRAY_T, 0,
-		CHARFLDSET(struct subscription_persistence, transport_key));
+		CHARFLDSET(struct subscription_persistence, transport_type));
 	ast_sorcery_object_field_register(sorcery, "subscription_persistence", "local_name", "", OPT_CHAR_ARRAY_T, 0,
 		CHARFLDSET(struct subscription_persistence, local_name));
 	ast_sorcery_object_field_register(sorcery, "subscription_persistence", "local_port", "0", OPT_UINT_T, 0,

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

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I1a20baf2a8643c272dcf819871d6c395f148f00b
Gerrit-Change-Number: 19640
Gerrit-PatchSet: 2
Gerrit-Owner: Friendly Automation
Gerrit-Reviewer: Friendly Automation
Gerrit-CC: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221203/99259be3/attachment-0001.html>


More information about the asterisk-code-review mailing list