]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
res_pjsip: Patch for res_pjsip_* module load/reload crash
authorSungtae Kim <pchero21@gmail.com>
Sun, 11 Nov 2018 16:29:10 +0000 (17:29 +0100)
committerCorey Farrell <git@cfware.com>
Wed, 21 Nov 2018 12:04:24 +0000 (07:04 -0500)
The session_supplements for the pjsip makes crashes when the module
load/unload.

ASTERISK-28157

Change-Id: I5b82be3a75d702cf1933d8d1417f44aa10ad1029

include/asterisk/res_pjsip_session.h
res/res_pjsip/pjsip_message_filter.c
res/res_pjsip/pjsip_session.c
res/res_pjsip_session.c

index ccee8c96d107d2a670db884fe179232e43822424..fbd1d1659a7937924bdcc0eaa13fb8cd49b24549 100644 (file)
@@ -580,9 +580,13 @@ void ast_sip_session_unregister_sdp_handler(struct ast_sip_session_sdp_handler *
  * set channel data based on headers in an incoming message. Similarly,
  * a module could reject an incoming request if desired.
  *
+ * \param module Referenced module(NULL safe)
  * \param supplement The supplement to register
  */
-void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement);
+void ast_sip_session_register_supplement_with_module(struct ast_module *module, struct ast_sip_session_supplement *supplement);
+
+#define ast_sip_session_register_supplement(supplement) \
+               ast_sip_session_register_supplement_with_module(AST_MODULE_SELF, supplement)
 
 /*!
  * \brief Unregister a an supplement to SIP session processing
@@ -598,6 +602,13 @@ void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *su
  */
 int ast_sip_session_add_supplements(struct ast_sip_session *session);
 
+/*!
+ * \brief Remove supplements from a SIP session
+ *
+ * \param session The session to remove
+ */
+void ast_sip_session_remove_supplements(struct ast_sip_session *session);
+
 /*!
  * \brief Alternative for ast_datastore_alloc()
  *
index f948c4449cc155fc3a23daf63165df16d569ff12..4b0f60dfc9e671c9a82ab4783678261933229466 100644 (file)
@@ -24,6 +24,7 @@
 #include "asterisk/res_pjsip.h"
 #include "asterisk/res_pjsip_session.h"
 #include "include/res_pjsip_private.h"
+#include "asterisk/module.h"
 
 #define MOD_DATA_RESTRICTIONS "restrictions"
 
index f3f3a4d8782bb401da7a14acc9c8d028b6e3a2fd..b9a25f5a4351b5474e4d19dbb81a931a075d8146 100644 (file)
 #include "asterisk/lock.h"
 #include "asterisk/module.h"
 
-
 AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);
 
-void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
+/* This structure is used to support module references without
+ * breaking the ABI of the public struct ast_sip_session_supplement. */
+struct private_sip_session_supplement {
+       /*! \brief This will be added to the session_supplements list.
+        *
+        * This field must be at offset 0 to enable retrieval of this
+        * structure from session_supplements.
+        */
+       struct ast_sip_session_supplement copy;
+       /*! Users of this session supplement will hold a reference to the module. */
+       struct ast_module *module;
+       /*! Pointer to the static variable for unregister comparisons. */
+       struct ast_sip_session_supplement *original;
+};
+
+void ast_sip_session_register_supplement_with_module(struct ast_module *module, struct ast_sip_session_supplement *supplement)
 {
        struct ast_sip_session_supplement *iter;
+       struct private_sip_session_supplement *priv;
        int inserted = 0;
        SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
 
+       ast_assert(supplement != NULL);
+
        if (!supplement->response_priority) {
                supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;
        }
 
+       priv = ast_calloc(1, sizeof(*priv));
+       if (!priv) {
+               /* Really can't do anything here, just assert and return. */
+               ast_assert(priv != NULL);
+               return;
+       }
+
+       priv->copy = *supplement;
+       priv->module = module;
+       priv->original = supplement;
+
        AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
                if (iter->priority > supplement->priority) {
-                       AST_RWLIST_INSERT_BEFORE_CURRENT(supplement, next);
+                       AST_RWLIST_INSERT_BEFORE_CURRENT(&priv->copy, next);
                        inserted = 1;
                        break;
                }
@@ -52,7 +80,7 @@ void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supp
        AST_RWLIST_TRAVERSE_SAFE_END;
 
        if (!inserted) {
-               AST_RWLIST_INSERT_TAIL(&session_supplements, supplement, next);
+               AST_RWLIST_INSERT_TAIL(&session_supplements, &priv->copy, next);
        }
 }
 
@@ -62,17 +90,20 @@ void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *su
        SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
 
        AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
-               if (supplement == iter) {
+               struct private_sip_session_supplement *priv = (void*)iter;
+
+               if (supplement == priv->original) {
                        AST_RWLIST_REMOVE_CURRENT(next);
+                       ast_free(priv);
                        break;
                }
        }
        AST_RWLIST_TRAVERSE_SAFE_END;
 }
 
-static struct ast_sip_session_supplement *supplement_dup(const struct ast_sip_session_supplement *src)
+static struct private_sip_session_supplement *supplement_dup(const struct private_sip_session_supplement *src)
 {
-       struct ast_sip_session_supplement *dst = ast_calloc(1, sizeof(*dst));
+       struct private_sip_session_supplement *dst = ast_calloc(1, sizeof(*dst));
 
        if (!dst) {
                return NULL;
@@ -89,13 +120,47 @@ int ast_sip_session_add_supplements(struct ast_sip_session *session)
        SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
 
        AST_RWLIST_TRAVERSE(&session_supplements, iter, next) {
-               struct ast_sip_session_supplement *copy = supplement_dup(iter);
+               struct private_sip_session_supplement *dup = supplement_dup((void*)iter);
 
-               if (!copy) {
+               if (!dup) {
                        return -1;
                }
-               AST_LIST_INSERT_TAIL(&session->supplements, copy, next);
+
+               /* referenced session created. increasing module reference. */
+               ast_module_ref(dup->module);
+
+               AST_LIST_INSERT_TAIL(&session->supplements, &dup->copy, next);
        }
 
        return 0;
 }
+
+void ast_sip_session_remove_supplements(struct ast_sip_session *session)
+{
+       struct ast_sip_session_supplement *iter;
+
+       if (!session) {
+               return;
+       }
+
+       /* free the supplements */
+       while ((iter = AST_LIST_REMOVE_HEAD(&session->supplements, next))) {
+               struct private_sip_session_supplement *priv = (void*)iter;
+               if (priv->module) {
+                       /* referenced session closed. decreasing module reference. */
+                       ast_module_unref(priv->module);
+               }
+
+               ast_free(iter);
+       }
+
+       return;
+}
+
+/* This stub is for ABI compatibility. */
+#undef ast_sip_session_register_supplement
+void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement);
+void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
+{
+       ast_sip_session_register_supplement_with_module(NULL, supplement);
+}
index 13c123ae10ca55fbc2b8de4d0c450df2d9d3bce8..f981cd98f1af762466aaa46b82fd4beb3157221d 100644 (file)
@@ -58,6 +58,9 @@
 #define DEFAULT_NUM_SESSION_MEDIA 2
 
 /* Some forward declarations */
+static void handle_session_begin(struct ast_sip_session *session);
+static void handle_session_end(struct ast_sip_session *session);
+static void handle_session_destroy(struct ast_sip_session *session);
 static void handle_incoming_request(struct ast_sip_session *session, pjsip_rx_data *rdata);
 static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata,
                enum ast_sip_session_response_priority response_priority);
@@ -2088,7 +2091,6 @@ static int datastore_cmp(void *obj, void *arg, int flags)
 static void session_destructor(void *obj)
 {
        struct ast_sip_session *session = obj;
-       struct ast_sip_session_supplement *supplement;
        struct ast_sip_session_delayed_request *delay;
        const char *endpoint_name = session->endpoint ?
                ast_sorcery_object_get_id(session->endpoint) : "<none>";
@@ -2104,19 +2106,18 @@ static void session_destructor(void *obj)
                , session->contact ? ast_sorcery_object_get_id(session->contact) : "<none>"
                );
 
-       while ((supplement = AST_LIST_REMOVE_HEAD(&session->supplements, next))) {
-               if (supplement->session_destroy) {
-                       supplement->session_destroy(session);
-               }
-               ast_free(supplement);
-       }
+       /* fire session destroy handler */
+       handle_session_destroy(session);
+
+       /* remove all registered supplements */
+       ast_sip_session_remove_supplements(session);
+       AST_LIST_HEAD_DESTROY(&session->supplements);
 
        ast_taskprocessor_unreference(session->serializer);
        ao2_cleanup(session->datastores);
        ast_sip_session_media_state_free(session->active_media_state);
        ast_sip_session_media_state_free(session->pending_media_state);
 
-       AST_LIST_HEAD_DESTROY(&session->supplements);
        while ((delay = AST_LIST_REMOVE_HEAD(&session->delayed_requests, next))) {
                delayed_request_free(delay);
        }
@@ -2165,7 +2166,6 @@ struct ast_sip_session *ast_sip_session_alloc(struct ast_sip_endpoint *endpoint,
 {
        RAII_VAR(struct ast_sip_session *, session, NULL, ao2_cleanup);
        struct ast_sip_session *ret_session;
-       struct ast_sip_session_supplement *iter;
        int dsp_features = 0;
 
        session = ao2_alloc(sizeof(*session), session_destructor);
@@ -2246,11 +2246,9 @@ struct ast_sip_session *ast_sip_session_alloc(struct ast_sip_endpoint *endpoint,
                ao2_ref(session, -1);
                return NULL;
        }
-       AST_LIST_TRAVERSE(&session->supplements, iter, next) {
-               if (iter->session_begin) {
-                       iter->session_begin(session);
-               }
-       }
+
+       /* Fire seesion begin handlers */
+       handle_session_begin(session);
 
        /* Avoid unnecessary ref manipulation to return a session */
        ret_session = session;
@@ -3315,6 +3313,40 @@ static void handle_incoming_request(struct ast_sip_session *session, pjsip_rx_da
        }
 }
 
+static void handle_session_begin(struct ast_sip_session *session)
+{
+       struct ast_sip_session_supplement *iter;
+
+       AST_LIST_TRAVERSE(&session->supplements, iter, next) {
+               if (iter->session_begin) {
+                       iter->session_begin(session);
+               }
+       }
+}
+
+static void handle_session_destroy(struct ast_sip_session *session)
+{
+       struct ast_sip_session_supplement *iter;
+
+       AST_LIST_TRAVERSE(&session->supplements, iter, next) {
+               if (iter->session_destroy) {
+                       iter->session_destroy(session);
+               }
+       }
+}
+
+static void handle_session_end(struct ast_sip_session *session)
+{
+       struct ast_sip_session_supplement *iter;
+
+       /* Session is dead.  Notify the supplements. */
+       AST_LIST_TRAVERSE(&session->supplements, iter, next) {
+               if (iter->session_end) {
+                       iter->session_end(session);
+               }
+       }
+}
+
 static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata,
                enum ast_sip_session_response_priority response_priority)
 {
@@ -3387,17 +3419,13 @@ static void handle_outgoing_response(struct ast_sip_session *session, pjsip_tx_d
 static int session_end(void *vsession)
 {
        struct ast_sip_session *session = vsession;
-       struct ast_sip_session_supplement *iter;
 
        /* Stop the scheduled termination */
        sip_session_defer_termination_stop_timer(session);
 
        /* Session is dead.  Notify the supplements. */
-       AST_LIST_TRAVERSE(&session->supplements, iter, next) {
-               if (iter->session_end) {
-                       iter->session_end(session);
-               }
-       }
+       handle_session_end(session);
+
        return 0;
 }