[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-commits
Subject: [asterisk-commits] =?utf-8?q?pjsip_transport_events=2Ec=3A_Fix_cr?= =?utf-8?q?ash_using_stale_transp
From: SVN commits to the Asterisk project <asterisk-commits () lists ! digium ! com>
Date: 2018-03-29 20:13:46
Message-ID: mailman.146.1522354476.1271.asterisk-commits () lists ! digium ! com
[Download RAW message or body]
Kevin Harwell has submitted this change and it was merged. ( \
https://gerrit.asterisk.org/8698 )
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:
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/8698
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia9b5469fea8f2b3f2d8476fae6b748a4d23e7261
Gerrit-Change-Number: 8698
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett@digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com>
Gerrit-Reviewer: Ross Beer <ross.beer@voicehost.co.uk>
Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com>
[Attachment #3 (text/html)]
<p>Kevin Harwell <strong>merged</strong> this change.</p><p><a \
href="https://gerrit.asterisk.org/8698">View Change</a></p><div \
style="white-space:pre-wrap">Approvals: Sean Bright: Looks good to me, but someone \
else must approve Kevin Harwell: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: \
pre-wrap;">pjsip_transport_events.c: Fix crash using stale transport \
pointer.<br><br>Apparently it is possible for the transport to be destroyed \
without<br>triggering the transport callback logic. As a result the transport \
gets<br>destroyed and we have a stale pointer in the active_transports \
container.<br><br>* Invoke the transport monitor callback checks when the transport \
is<br>destroyed in addition to when it is disconnected and \
shutdown.<br><br>ASTERISK-27688<br><br>Change-Id: \
Ia9b5469fea8f2b3f2d8476fae6b748a4d23e7261<br>---<br>M \
res/res_pjsip/pjsip_transport_events.c<br>1 file changed, 40 insertions(+), 15 \
deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: \
pre-wrap;">diff --git a/res/res_pjsip/pjsip_transport_events.c \
b/res/res_pjsip/pjsip_transport_events.c<br>index c701b84..cc7b7c0 100644<br>--- \
a/res/res_pjsip/pjsip_transport_events.c<br>+++ \
b/res/res_pjsip/pjsip_transport_events.c<br>@@ -114,6 +114,36 @@<br> \
AST_VECTOR_FREE(&monitored->monitors);<br> }<br> <br>+/*!<br>+ * \
\internal<br>+ * \brief Do registered callbacks for the transport.<br>+ * \since \
13.21.0<br>+ *<br>+ * \param transports Active transports container<br>+ * \param \
transport Which transport to do callbacks for.<br>+ *<br>+ * \return Nothing<br>+ \
*/<br>+static void transport_state_do_reg_callbacks(struct ao2_container *transports, \
pjsip_transport *transport)<br>+{<br>+ struct transport_monitor \
*monitored;<br>+<br>+ monitored = ao2_find(transports, transport->obj_name, \
OBJ_SEARCH_KEY | OBJ_UNLINK);<br>+ if (monitored) {<br>+ int idx;<br>+<br>+ for \
(idx = AST_VECTOR_SIZE(&monitored->monitors); idx--;) {<br>+ struct \
transport_monitor_notifier *notifier;<br>+<br>+ notifier = \
AST_VECTOR_GET_ADDR(&monitored->monitors, idx);<br>+ ast_debug(3, \
"running callback %p(%p) for transport %s\n",<br>+ notifier->cb, \
notifier->data, transport->obj_name);<br>+ notifier->cb(notifier->data);<br>+ }<br>+ ao2_ref(monitored, \
-1);<br>+ }<br>+}<br>+<br> /*! \brief Callback invoked when transport state changes \
occur */<br> static void transport_state_callback(pjsip_transport *transport,<br> \
pjsip_transport_state state, const pjsip_transport_state_info *info)<br>@@ -147,6 \
+177,7 @@<br> if (!transport->is_shutdown) {<br> \
pjsip_transport_shutdown(transport);<br> \
}<br>+ transport_state_do_reg_callbacks(transports, transport);<br> break;<br> \
case PJSIP_TP_STATE_SHUTDOWN:<br> /*<br>@@ -157,23 +188,17 @@<br> */<br> \
transport->is_shutdown = PJ_TRUE;<br> <br>- monitored = ao2_find(transports, \
transport->obj_name,<br>- OBJ_SEARCH_KEY | OBJ_UNLINK);<br>- if (monitored) \
{<br>- int idx;<br>-<br>- for (idx = \
AST_VECTOR_SIZE(&monitored->monitors); idx--;) {<br>- struct \
transport_monitor_notifier *notifier;<br>-<br>- notifier = \
AST_VECTOR_GET_ADDR(&monitored->monitors, idx);<br>- ast_debug(3, \
"running callback %p(%p) for transport %s\n",<br>- notifier->cb, \
notifier->data, transport->obj_name);<br>- notifier->cb(notifier->data);<br>- }<br>- ao2_ref(monitored, \
-1);<br>- }<br>+ transport_state_do_reg_callbacks(transports, \
transport);<br>+ break;<br>+ case \
PJSIP_TP_STATE_DESTROY:<br>+ transport_state_do_reg_callbacks(transports, \
transport);<br> break;<br> default:<br>+ /*<br>+ * We have to have a \
default case because the enum is<br>+ * defined by a third-party library.<br>+ \
*/<br>+ ast_assert(0);<br> break;<br> }<br> <br></pre><p>To view, visit <a \
href="https://gerrit.asterisk.org/8698">change 8698</a>. To unsubscribe, visit <a \
href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope \
itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" \
itemtype="http://schema.org/ViewAction"><link itemprop="url" \
href="https://gerrit.asterisk.org/8698"/><meta itemprop="name" content="View \
Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: \
Ia9b5469fea8f2b3f2d8476fae6b748a4d23e7261 </div> <div style="display:none"> \
Gerrit-Change-Number: 8698 </div> <div style="display:none"> Gerrit-PatchSet: 1 \
</div> <div style="display:none"> Gerrit-Owner: Richard Mudgett \
<rmudgett@digium.com> </div> <div style="display:none"> Gerrit-Reviewer: \
Jenkins2 </div> <div style="display:none"> Gerrit-Reviewer: Kevin Harwell \
<kharwell@digium.com> </div> <div style="display:none"> Gerrit-Reviewer: Ross \
Beer <ross.beer@voicehost.co.uk> </div> <div style="display:none"> \
Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-commits mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-commits
--===============7604329071729603449==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic