res_pjsip: Move URI validation to use time.

In a realtime based system with a limited number of threadpool threads
it is possible for a deadlock to occur. This happens when permanent
endpoint state is updated, which will cause database queries to be done.
These queries may result in URI validation being done which is done
synchronously using a PJSIP thread. If all PJSIP threads are in use
processing traffic they themselves may be blocked waiting to get the
permanent endpoint container lock when identifying an endpoint.

This change moves URI validation to occur at use time instead of
configuration time. While this comes at a cost of not seeing a problem
until you use it it does solve the underlying deadlock problem.

ASTERISK-25486 #close

Change-Id: I2d7d167af987d23b3e8199e4a68f3359eba4c76a

diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index a36935dd338204bc096fb58ff57409d83f5b3770..459082901f82308a150b7f88c4836b74eda276d8 100644 (file)
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -1190,6 +1190,11 @@ int ast_sip_push_task(struct ast_taskprocessor *serializer, int (*sip_task)(void
  * cause a deadlock. If you are in a SIP servant thread, just call your function
  * in-line.
  *
+ * \warning \b Never hold locks that may be acquired by a SIP servant thread when
+ * calling this function. Doing so may cause a deadlock if all SIP servant threads
+ * are blocked waiting to acquire the lock while the thread holding the lock is
+ * waiting for a free SIP servant thread.
+ *
  * \param serializer The SIP serializer to which the task belongs. May be NULL.
  * \param sip_task The task to execute
  * \param task_data The parameter to pass to the task when it executes

diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 9e5e404b626acab670ba39fd8d58bbfbf2bf3e4a..d2b393fccf930633e91362d24d377438676fd666 100644 (file)
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -2563,6 +2563,8 @@ pjsip_dialog *ast_sip_create_dialog_uac(const struct ast_sip_endpoint *endpoint,
 
                pj_strdup2_with_null(dlg->pool, &tmp, outbound_proxy);
                if (!(route = pjsip_parse_hdr(dlg->pool, &ROUTE_HNAME, tmp.ptr, tmp.slen, NULL))) {
+                       ast_log(LOG_ERROR, "Could not create dialog to endpoint '%s' as outbound proxy URI '%s' is not valid\n",
+                               ast_sorcery_object_get_id(endpoint), outbound_proxy);
                        dlg->sess_count--;
                        pjsip_dlg_terminate(dlg);
                        return NULL;
@@ -2756,6 +2758,7 @@ static int create_out_of_dialog_request(const pjsip_method *method, struct ast_s
        pj_str_t from;
        pj_pool_t *pool;
        pjsip_tpselector selector = { .type = PJSIP_TPSELECTOR_NONE, };
+       pjsip_uri *sip_uri;
 
        if (ast_strlen_zero(uri)) {
                if (!endpoint && (!contact || ast_strlen_zero(contact->uri))) {
@@ -2792,6 +2795,16 @@ static int create_out_of_dialog_request(const pjsip_method *method, struct ast_s
                return -1;
        }
 
+       sip_uri = pjsip_parse_uri(pool, remote_uri.ptr, remote_uri.slen, 0);
+       if (!sip_uri || (!PJSIP_URI_SCHEME_IS_SIP(sip_uri) && !PJSIP_URI_SCHEME_IS_SIPS(sip_uri))) {
+               ast_log(LOG_ERROR, "Unable to create outbound %.*s request to endpoint %s as URI '%s' is not valid\n",
+                       (int) pj_strlen(&method->name), pj_strbuf(&method->name),
+                       endpoint ? ast_sorcery_object_get_id(endpoint) : "<none>",
+                       pj_strbuf(&remote_uri));
+               pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+               return -1;
+       }
+
        if (sip_dialog_create_from(pool, &from, endpoint ? endpoint->fromuser : NULL,
                                endpoint ? endpoint->fromdomain : NULL, &remote_uri, &selector)) {
                ast_log(LOG_ERROR, "Unable to create From header for %.*s request to endpoint %s\n",
@@ -2816,8 +2829,9 @@ static int create_out_of_dialog_request(const pjsip_method *method, struct ast_s
        /* If an outbound proxy is specified on the endpoint apply it to this request */
        if (endpoint && !ast_strlen_zero(endpoint->outbound_proxy) &&
                ast_sip_set_outbound_proxy((*tdata), endpoint->outbound_proxy)) {
-               ast_log(LOG_ERROR, "Unable to apply outbound proxy on request %.*s to endpoint %s\n",
-                       (int) pj_strlen(&method->name), pj_strbuf(&method->name), ast_sorcery_object_get_id(endpoint));
+               ast_log(LOG_ERROR, "Unable to apply outbound proxy on request %.*s to endpoint %s as outbound proxy URI '%s' is not valid\n",
+                       (int) pj_strlen(&method->name), pj_strbuf(&method->name), ast_sorcery_object_get_id(endpoint),
+                       endpoint->outbound_proxy);
                pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
                return -1;
        }

diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 9625f04ef2ca4f394c2fb4c3a7e2864470c63d7b..331d839a1ebfb1cc6b719a4ee07afd0d6423fc60 100644 (file)
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -319,32 +319,6 @@ static int expiration_struct2str(const void *obj, const intptr_t *args, char **b
        return (ast_asprintf(buf, "%ld", contact->expiration_time.tv_sec) < 0) ? -1 : 0;
 }
 
-/*! \brief Helper function which validates a permanent contact */
-static int permanent_contact_validate(void *data)
-{
-       const char *value = data;
-       pj_pool_t *pool;
-       pj_str_t contact_uri;
-       static const pj_str_t HCONTACT = { "Contact", 7 };
-       pjsip_contact_hdr *contact_hdr;
-       int rc = 0;
-
-       pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "Permanent Contact Validation", 256, 256);
-       if (!pool) {
-               return -1;
-       }
-
-       pj_strdup2_with_null(pool, &contact_uri, value);
-       if (!(contact_hdr = pjsip_parse_hdr(pool, &HCONTACT, contact_uri.ptr, contact_uri.slen, NULL))
-               || !(PJSIP_URI_SCHEME_IS_SIP(contact_hdr->uri)
-                       || PJSIP_URI_SCHEME_IS_SIPS(contact_hdr->uri))) {
-               rc = -1;
-       }
-
-       pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
-       return rc;
-}
-
 static int permanent_uri_sort_fn(const void *obj_left, const void *obj_right, int flags)
 {
        const struct ast_sip_contact *object_left = obj_left;
@@ -393,12 +367,6 @@ static int permanent_uri_handler(const struct aco_option *opt, struct ast_variab
                struct ast_sip_contact_status *status;
                char contact_id[strlen(aor_id) + strlen(contact_uri) + 2 + 1];
 
-               if (ast_sip_push_task_synchronous(NULL, permanent_contact_validate, contact_uri)) {
-                       ast_log(LOG_ERROR, "Permanent URI on aor '%s' with contact '%s' failed to parse\n",
-                               ast_sorcery_object_get_id(aor), contact_uri);
-                       return -1;
-               }
-
                if (!aor->permanent_contacts) {
                        aor->permanent_contacts = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK,
                                AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT, permanent_uri_sort_fn, NULL);

diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 2fdfc9d0bb843237ac61253f69690273fa66a88d..bcbb791e63acaa7d4efc53b27e7f0e27c60bc1f0 100644 (file)
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1084,29 +1084,6 @@ static struct ast_endpoint *persistent_endpoint_find_or_create(const struct ast_
        return persistent->endpoint;
 }
 
-/*! \brief Helper function which validates an outbound proxy */
-static int outbound_proxy_validate(void *data)
-{
-       const char *proxy = data;
-       pj_pool_t *pool;
-       pj_str_t tmp;
-       static const pj_str_t ROUTE_HNAME = { "Route", 5 };
-
-       pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "Outbound Proxy Validation", 256, 256);
-       if (!pool) {
-               return -1;
-       }
-
-       pj_strdup2_with_null(pool, &tmp, proxy);
-       if (!pjsip_parse_hdr(pool, &ROUTE_HNAME, tmp.ptr, tmp.slen, NULL)) {
-               pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
-               return -1;
-       }
-
-       pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
-       return 0;
-}
-
 /*! \brief Callback function for when an object is finalized */
 static int sip_endpoint_apply_handler(const struct ast_sorcery *sorcery, void *obj)
 {
@@ -1116,12 +1093,7 @@ static int sip_endpoint_apply_handler(const struct ast_sorcery *sorcery, void *o
                return -1;
        }
 
-       if (!ast_strlen_zero(endpoint->outbound_proxy) &&
-               ast_sip_push_task_synchronous(NULL, outbound_proxy_validate, (char*)endpoint->outbound_proxy)) {
-               ast_log(LOG_ERROR, "Invalid outbound proxy '%s' specified on endpoint '%s'\n",
-                       endpoint->outbound_proxy, ast_sorcery_object_get_id(endpoint));
-               return -1;
-       } else if (endpoint->extensions.timer.min_se < 90) {
+       if (endpoint->extensions.timer.min_se < 90) {
                ast_log(LOG_ERROR, "Session timer minimum expires time must be 90 or greater on endpoint '%s'\n",
                        ast_sorcery_object_get_id(endpoint));
                return -1;