From b6b49071afba5ceaaa214e2b62b661e5fed62418 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Tue, 3 Jul 2018 12:10:36 -0500 Subject: [PATCH] res_pjsip/pjsip_transport_management.c: Fix deadlock with transport keep alive. Using the keep_alive_interval option can result in a deadlock between the pjproject transport manager group lock and the monitored transports ao2 container lock. The pjproject transport manager group lock has to be superior in the locking order to the monitored transports ao2 container lock because of pjproject callbacks called when already holding the group lock. The lock inversion happens when Asterisk attempts to send a keep alive packet over the reliable transports. * Made keepalive_transport_thread() iterate over the monitored transports container rather than use the ao2_callback() method. This avoids holding the container lock when sending the keep alive packet. ASTERISK-26686 Change-Id: I5d5392a52e698bbe41a93f7d8e92bf0e61fe3951 --- res/res_pjsip/pjsip_transport_management.c | 35 +++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/res/res_pjsip/pjsip_transport_management.c b/res/res_pjsip/pjsip_transport_management.c index 1404b6eee4..d9866f1644 100644 --- a/res/res_pjsip/pjsip_transport_management.c +++ b/res/res_pjsip/pjsip_transport_management.c @@ -56,21 +56,22 @@ struct monitored_transport { int sip_received; }; -/*! \brief Callback function to send keepalive */ -static int keepalive_transport_cb(void *obj, void *arg, int flags) +static void keepalive_transport_send_keepalive(struct monitored_transport *monitored) { - struct monitored_transport *monitored = obj; pjsip_tpselector selector = { .type = PJSIP_TPSELECTOR_TRANSPORT, .u.transport = monitored->transport, }; pjsip_tpmgr_send_raw(pjsip_endpt_get_tpmgr(ast_sip_get_pjsip_endpoint()), - monitored->transport->key.type, &selector, NULL, keepalive_packet.ptr, keepalive_packet.slen, - &monitored->transport->key.rem_addr, pj_sockaddr_get_len(&monitored->transport->key.rem_addr), + monitored->transport->key.type, + &selector, + NULL, + keepalive_packet.ptr, + keepalive_packet.slen, + &monitored->transport->key.rem_addr, + pj_sockaddr_get_len(&monitored->transport->key.rem_addr), NULL, NULL); - - return 0; } /*! \brief Thread which sends keepalives to all active connection-oriented transports */ @@ -90,12 +91,26 @@ static void *keepalive_transport_thread(void *data) return NULL; } - /* Once loaded this module just keeps on going as it is unsafe to stop and change the underlying - * callback for the transport manager. + /* + * Once loaded this module just keeps on going as it is unsafe to stop + * and change the underlying callback for the transport manager. */ while (keepalive_interval) { + struct ao2_iterator iter; + struct monitored_transport *monitored; + sleep(keepalive_interval); - ao2_callback(transports, OBJ_NODATA, keepalive_transport_cb, NULL); + + /* + * We must use the iterator to avoid deadlock between the container lock + * and the pjproject transport manager group lock when sending + * the keepalive packet. + */ + iter = ao2_iterator_init(transports, 0); + for (; (monitored = ao2_iterator_next(&iter)); ao2_ref(monitored, -1)) { + keepalive_transport_send_keepalive(monitored); + } + ao2_iterator_destroy(&iter); } ao2_ref(transports, -1);