Stasis: Fix unsafe use of stasis_unsubscribe in modules.

Many uses of stasis_unsubscribe in modules can be reached through unload.
These have been switched to stasis_unsubscribe_and_join.

Some subscription callbacks do nothing, for these I've created a noop
callback function in stasis.c.  This is used by some modules that monitor
MWI topics in order to enable cache, since the callback does not become
invalid after dlclose it is safe to use stasis_unsubscribe on these, even
during module unload.

ASTERISK-25121 #close

Change-Id: Ifc2549fbd8eef7d703c222978e8f452e2972189c

diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index 6958e5be540b781b5aabf79332266e7a5b9beb40..953561ce4a814e09f0940862fda0636adea8ec7c 100644 (file)
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -600,14 +600,6 @@ static int restart_monitor(void);
 
 static int dahdi_sendtext(struct ast_channel *c, const char *text);
 
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
-       /* This module does not handle MWI in an event-based manner.  However, it
-        * subscribes to MWI for each mailbox that is configured so that the core
-        * knows that we care about it.  Then, chan_dahdi will get the MWI from the
-        * event cache instead of checking the mailbox directly. */
-}
-
 /*! \brief Avoid the silly dahdi_getevent which ignores a bunch of events */
 static inline int dahdi_get_event(int fd)
 {
@@ -12594,7 +12586,11 @@ static struct dahdi_pvt *mkintf(int channel, const struct dahdi_chan_conf *conf,
 
                        mailbox_specific_topic = ast_mwi_topic(tmp->mailbox);
                        if (mailbox_specific_topic) {
-                               tmp->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, mwi_event_cb, NULL);
+                               /* This module does not handle MWI in an event-based manner.  However, it
+                                * subscribes to MWI for each mailbox that is configured so that the core
+                                * knows that we care about it.  Then, chan_dahdi will get the MWI from the
+                                * event cache instead of checking the mailbox directly. */
+                               tmp->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, stasis_subscription_cb_noop, NULL);
                        }
                }
 #ifdef HAVE_DAHDI_LINEREVERSE_VMWI

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index a5f86e1887887bdc732e47e5cf702171487bb977..dc30d9fe7cf3ad57958b1e588499dec4319316fd 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -1438,13 +1438,6 @@ static int iax2_is_control_frame_allowed(int subtype)
        return is_allowed;
 }
 
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
-       /* The MWI subscriptions exist just so the core knows we care about those
-        * mailboxes.  However, we just grab the events out of the cache when it
-        * is time to send MWI, since it is only sent with a REGACK. */
-}
-
 static void network_change_stasis_subscribe(void)
 {
        if (!network_change_sub) {
@@ -13029,7 +13022,10 @@ static struct iax2_peer *build_peer(const char *name, struct ast_variable *v, st
 
                mailbox_specific_topic = ast_mwi_topic(peer->mailbox);
                if (mailbox_specific_topic) {
-                       peer->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, mwi_event_cb, NULL);
+                       /* The MWI subscriptions exist just so the core knows we care about those
+                        * mailboxes.  However, we just grab the events out of the cache when it
+                        * is time to send MWI, since it is only sent with a REGACK. */
+                       peer->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, stasis_subscription_cb_noop, NULL);
                }
        }
 

diff --git a/channels/chan_mgcp.c b/channels/chan_mgcp.c
index 08c4dc2e20fb0e138ffa080782f8dd91fadfbbde..b75cd43afb1259c2a04b873f9abddcbf5d414315 100644 (file)
--- a/channels/chan_mgcp.c
+++ b/channels/chan_mgcp.c
@@ -489,14 +489,6 @@ static struct ast_channel_tech mgcp_tech = {
        .func_channel_read = acf_channel_read,
 };
 
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
-       /* This module does not handle MWI in an event-based manner.  However, it
-        * subscribes to MWI for each mailbox that is configured so that the core
-        * knows that we care about it.  Then, chan_mgcp will get the MWI from the
-        * event cache instead of checking the mailbox directly. */
-}
-
 static int has_voicemail(struct mgcp_endpoint *p)
 {
        int new_msgs;
@@ -4237,7 +4229,11 @@ static struct mgcp_gateway *build_gateway(char *cat, struct ast_variable *v)
 
                                        mailbox_specific_topic = ast_mwi_topic(e->mailbox);
                                        if (mailbox_specific_topic) {
-                                               e->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, mwi_event_cb, NULL);
+                                               /* This module does not handle MWI in an event-based manner.  However, it
+                                                * subscribes to MWI for each mailbox that is configured so that the core
+                                                * knows that we care about it.  Then, chan_mgcp will get the MWI from the
+                                                * event cache instead of checking the mailbox directly. */
+                                               e->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, stasis_subscription_cb_noop, NULL);
                                        }
                                }
                                snprintf(e->rqnt_ident, sizeof(e->rqnt_ident), "%08lx", (unsigned long)ast_random());

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 1307a93427593e8a85ac18c563c0b36f03536263..b1fbd6e9b6f72051755a536dd9430053c806e0cd 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -5004,7 +5004,7 @@ static void register_peer_exten(struct sip_peer *peer, int onoff)
 static void destroy_mailbox(struct sip_mailbox *mailbox)
 {
        if (mailbox->event_sub) {
-               mailbox->event_sub = stasis_unsubscribe(mailbox->event_sub);
+               mailbox->event_sub = stasis_unsubscribe_and_join(mailbox->event_sub);
        }
        ast_free(mailbox);
 }

diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index ed82c7d1fcfb941bec732a70d6444c2bed914096..be9f9b67ef5a3981681ec48c06e43a50c1421bfa 100644 (file)
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -8749,7 +8749,7 @@ static int unload_module(void)
                                skinny_unlocksub(sub);
                        }
                        if (l->mwi_event_sub) {
-                               l->mwi_event_sub = stasis_unsubscribe(l->mwi_event_sub);
+                               l->mwi_event_sub = stasis_unsubscribe_and_join(l->mwi_event_sub);
                        }
                        ast_mutex_unlock(&l->lock);
                        unregister_exten(l);

diff --git a/channels/sig_pri.c b/channels/sig_pri.c
index 2815dac1342766791e37f8889a342e4fc6d3d8c8..4b913910160c8f1e2f0b4c8abbef0ca894dd2913 100644 (file)
--- a/channels/sig_pri.c
+++ b/channels/sig_pri.c
@@ -8991,7 +8991,7 @@ void sig_pri_stop_pri(struct sig_pri_span *pri)
 #if defined(HAVE_PRI_MWI)
        for (idx = 0; idx < ARRAY_LEN(pri->mbox); ++idx) {
                if (pri->mbox[idx].sub) {
-                       pri->mbox[idx].sub = stasis_unsubscribe(pri->mbox[idx].sub);
+                       pri->mbox[idx].sub = stasis_unsubscribe_and_join(pri->mbox[idx].sub);
                }
        }
 #endif /* defined(HAVE_PRI_MWI) */

diff --git a/include/asterisk/stasis.h b/include/asterisk/stasis.h
index 0b1b1e83f88ca01f5969af5c7993a794d0816af2..16b30ccb3845803f29591ce74bf16cfffe538f1f 100644 (file)
--- a/include/asterisk/stasis.h
+++ b/include/asterisk/stasis.h
@@ -520,6 +520,17 @@ void stasis_publish_sync(struct stasis_subscription *sub, struct stasis_message
  */
 typedef void (*stasis_subscription_cb)(void *data, struct stasis_subscription *sub, struct stasis_message *message);
 
+/*!
+ * \brief Stasis subscription callback function that does nothing.
+ *
+ * \note This callback should be used for events are not directly processed, but need
+ * to be generated so data can be retrieved from cache later.  Subscriptions with this
+ * callback can be released with \ref stasis_unsubscribe, even during module unload.
+ *
+ * \since 13.5
+ */
+void stasis_subscription_cb_noop(void *data, struct stasis_subscription *sub, struct stasis_message *message);
+
 /*!
  * \brief Create a subscription.
  *

diff --git a/main/stasis.c b/main/stasis.c
index c2d552e4bebd5a91b628782a9f668ca7bd6b242e..6adbfc3b69383e4b2214fe0f66a89bc89334182a 100644 (file)
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -444,6 +444,10 @@ static void subscription_invoke(struct stasis_subscription *sub,
 static void send_subscription_subscribe(struct stasis_topic *topic, struct stasis_subscription *sub);
 static void send_subscription_unsubscribe(struct stasis_topic *topic, struct stasis_subscription *sub);
 
+void stasis_subscription_cb_noop(void *data, struct stasis_subscription *sub, struct stasis_message *message)
+{
+}
+
 struct stasis_subscription *internal_stasis_subscribe(
        struct stasis_topic *topic,
        stasis_subscription_cb callback,

diff --git a/res/res_hep_rtcp.c b/res/res_hep_rtcp.c
index fe39f195b53dd07cc4a21e57979e2ac82c4a4fd8..787512bbfe3d963fe45d172f70e6b9dd9ce6200e 100644 (file)
--- a/res/res_hep_rtcp.c
+++ b/res/res_hep_rtcp.c
@@ -131,7 +131,7 @@ static int load_module(void)
 static int unload_module(void)
 {
        if (stasis_rtp_subscription) {
-               stasis_rtp_subscription = stasis_unsubscribe(stasis_rtp_subscription);
+               stasis_rtp_subscription = stasis_unsubscribe_and_join(stasis_rtp_subscription);
        }
 
        return 0;

diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index 2ab7dfee0ea3292bcffd520bc61361cc9b5da2ca..76e0e4c3513eefcf42e00f3c7e08bee767bb2e0c 100644 (file)
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -471,7 +471,7 @@ static int unsubscribe_stasis(void *obj, void *arg, int flags)
        struct mwi_stasis_subscription *mwi_stasis = obj;
        if (mwi_stasis->stasis_sub) {
                ast_debug(3, "Removing stasis subscription to mailbox %s\n", mwi_stasis->mailbox);
-               mwi_stasis->stasis_sub = stasis_unsubscribe(mwi_stasis->stasis_sub);
+               mwi_stasis->stasis_sub = stasis_unsubscribe_and_join(mwi_stasis->stasis_sub);
        }
        return CMP_MATCH;
 }

diff --git a/res/res_security_log.c b/res/res_security_log.c
index 78ca1218cd7552eb4465d98353c0d384b3ec47d1..c3fb3cfd2de38640786a17d1df48bf3dcc7c7e15 100644 (file)
--- a/res/res_security_log.c
+++ b/res/res_security_log.c
@@ -152,7 +152,7 @@ static int load_module(void)
 static int unload_module(void)
 {
        if (security_stasis_sub) {
-               security_stasis_sub = stasis_unsubscribe(security_stasis_sub);
+               security_stasis_sub = stasis_unsubscribe_and_join(security_stasis_sub);
        }
 
        ast_logger_unregister_level(LOG_SECURITY_NAME);

diff --git a/res/res_stasis_device_state.c b/res/res_stasis_device_state.c
index 8a1c230491864689ef70f8c84b0965666f754cf4..aec6a6eca8d0b054215e8aec4e31d3a463f3dd8e 100644 (file)
--- a/res/res_stasis_device_state.c
+++ b/res/res_stasis_device_state.c
@@ -105,7 +105,7 @@ static int device_state_subscriptions_cmp(void *obj, void *arg, int flags)
 static void device_state_subscription_destroy(void *obj)
 {
        struct device_state_subscription *sub = obj;
-       sub->sub = stasis_unsubscribe(sub->sub);
+       sub->sub = stasis_unsubscribe_and_join(sub->sub);
        ast_string_field_free_memory(sub);
 }
 

diff --git a/res/res_xmpp.c b/res/res_xmpp.c
index 9d21297b04f324873bed7eca7b1da04cdf52b128..d9791431d502953635f5c143839db9e9595b8ecd 100644 (file)
--- a/res/res_xmpp.c
+++ b/res/res_xmpp.c
@@ -3568,12 +3568,12 @@ int ast_xmpp_client_disconnect(struct ast_xmpp_client *client)
        }
 
        if (client->mwi_sub) {
-               client->mwi_sub = stasis_unsubscribe(client->mwi_sub);
+               client->mwi_sub = stasis_unsubscribe_and_join(client->mwi_sub);
                xmpp_pubsub_unsubscribe(client, "message_waiting");
        }
 
        if (client->device_state_sub) {
-               client->device_state_sub = stasis_unsubscribe(client->device_state_sub);
+               client->device_state_sub = stasis_unsubscribe_and_join(client->device_state_sub);
                xmpp_pubsub_unsubscribe(client, "device_state");
        }