[Asterisk-code-review] pjsip transport events.c: Fix crash using stale transport po... (asterisk[13])

[Kevin Harwell]

Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/8455 )

Change subject: pjsip_transport_events.c: Fix crash using stale transport pointer.
......................................................................

pjsip_transport_events.c: Fix crash using stale transport pointer.

Apparently it is possible for the transport to be destroyed without
triggering the transport callback logic.  As a result the transport gets
destroyed and we have a stale pointer in the active_transports container.

* Invoke the transport monitor callback checks when the transport is
destroyed in addition to when it is disconnected and shutdown.

ASTERISK-27688

Change-Id: Ia9b5469fea8f2b3f2d8476fae6b748a4d23e7261
---
M res/res_pjsip/pjsip_transport_events.c
1 file changed, 40 insertions(+), 15 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Sean Bright: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip/pjsip_transport_events.c b/res/res_pjsip/pjsip_transport_events.c
index c701b84..cc7b7c0 100644
--- a/res/res_pjsip/pjsip_transport_events.c
+++ b/res/res_pjsip/pjsip_transport_events.c
@@ -114,6 +114,36 @@
 	AST_VECTOR_FREE(&monitored->monitors);
 }
 
+/*!
+ * \internal
+ * \brief Do registered callbacks for the transport.
+ * \since 13.21.0
+ *
+ * \param transports Active transports container
+ * \param transport Which transport to do callbacks for.
+ *
+ * \return Nothing
+ */
+static void transport_state_do_reg_callbacks(struct ao2_container *transports, pjsip_transport *transport)
+{
+	struct transport_monitor *monitored;
+
+	monitored = ao2_find(transports, transport->obj_name, OBJ_SEARCH_KEY | OBJ_UNLINK);
+	if (monitored) {
+		int idx;
+
+		for (idx = AST_VECTOR_SIZE(&monitored->monitors); idx--;) {
+			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);
+			notifier->cb(notifier->data);
+		}
+		ao2_ref(monitored, -1);
+	}
+}
+
 /*! \brief Callback invoked when transport state changes occur */
 static void transport_state_callback(pjsip_transport *transport,
 	pjsip_transport_state state, const pjsip_transport_state_info *info)
@@ -147,6 +177,7 @@
 			if (!transport->is_shutdown) {
 				pjsip_transport_shutdown(transport);
 			}
+			transport_state_do_reg_callbacks(transports, transport);
 			break;
 		case PJSIP_TP_STATE_SHUTDOWN:
 			/*
@@ -157,23 +188,17 @@
 			 */
 			transport->is_shutdown = PJ_TRUE;
 
-			monitored = ao2_find(transports, transport->obj_name,
-				OBJ_SEARCH_KEY | OBJ_UNLINK);
-			if (monitored) {
-				int idx;
-
-				for (idx = AST_VECTOR_SIZE(&monitored->monitors); idx--;) {
-					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);
-					notifier->cb(notifier->data);
-				}
-				ao2_ref(monitored, -1);
-			}
+			transport_state_do_reg_callbacks(transports, transport);
+			break;
+		case PJSIP_TP_STATE_DESTROY:
+			transport_state_do_reg_callbacks(transports, transport);
 			break;
 		default:
+			/*
+			 * We have to have a default case because the enum is
+			 * defined by a third-party library.
+			 */
+			ast_assert(0);
 			break;
 		}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia9b5469fea8f2b3f2d8476fae6b748a4d23e7261
Gerrit-Change-Number: 8455
Gerrit-PatchSet: 4
Gerrit-Owner: Ross Beer <ross.beer at voicehost.co.uk>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180329/314ea4ec/attachment-0001.html>