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 2f637dcfe16e39b698ed1b9cbef622989682d589..fe613097c9e878dc4e27148a28fcd425caa3afae 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)
 {
@@ -12593,7 +12585,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 0c27d2d63e069eedba96a2016c798059d386ba82..dbad79dcc206942b5982a2275672f019c7ce6e04 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -1430,13 +1430,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) {
@@ -13010,7 +13003,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 98c6c307b7bd28756d49bba40f34f3c7254c58e2..16d3c6550e51f4ec27d4157bedf97aa167fdab2d 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;
@@ -4249,7 +4241,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 9eeb75fe3ec37a52225f82910b69c42123aa8ef7..7c4c8a6113ca0349b74429e917fd5dbbdd979434 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -5008,7 +5008,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 47c7352c6611886b9d1687cbc5bbfed640b8009a..03da0e0d8b6b140f4c43f96cac54b37575a89c93 100644 (file)
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -8756,7 +8756,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 71e7e23deb5502d3c2199d71d8c4f73f7cef191a..b009c4520e8881dc79f179efcc1eb6ecaaae0d49 100644 (file)
--- a/channels/sig_pri.c
+++ b/channels/sig_pri.c
@@ -8978,7 +8978,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 aa681e13ea8b9c1b11f2b80043b7893a7e0b343f..69b2d0f2267421a50370931883a561618638bcf9 100644 (file)
--- a/include/asterisk/stasis.h
+++ b/include/asterisk/stasis.h
@@ -510,6 +510,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 6a59265460cdec4e66c6f8fc4d069a7901f8b902..e168ce93b0850bc458d59505807bfab29269c573 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 352262b6fada5dc7336722227a02975b581cc202..25aed152077b626ca22c29ee7b03328c3c2bfaa8 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 8fefc3ae0279f63692cbba2bdae47f3e9d0cbe52..371f3abf857763aaffe25e147656c07ec228d27b 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 d32b32c9e83577798b27b075cbba657624419bed..94a78d8039d9f74d7ee3bdf0744e6441b826aa69 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 1d135fe55d306d6c40fdbceebe0e75654d55ec7b..7f7c513649903fc4980bb0f509161a0af9b44bf5 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 5a3670e7d63a1952b5cf396c94a1887ecdedd41f..2a087b055b9d90eeeaa0faba0f32ceb4c2a72ebb 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");
        }