Merge "Stasis: Fix unsafe use of stasis_unsubscribe in modules." into 13

13 files changed, 38 insertions, 35 deletions

diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index 6958e5be5..953561ce4 100644
--- 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 a5f86e188..dc30d9fe7 100644
--- 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 08c4dc2e2..b75cd43af 100644
--- 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 1307a9342..b1fbd6e9b 100644
--- 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 ed82c7d1f..be9f9b67e 100644
--- 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 2815dac13..4b9139101 100644
--- 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 0b1b1e83f..16b30ccb3 100644
--- a/include/asterisk/stasis.h
+++ b/include/asterisk/stasis.h
@@ -521,6 +521,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.
  *
  * In addition to being AO2 managed memory (requiring an ao2_cleanup() to free
diff --git a/main/stasis.c b/main/stasis.c
index c2d552e4b..6adbfc3b6 100644
--- 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 fe39f195b..787512bbf 100644
--- 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 2ab7dfee0..76e0e4c35 100644
--- 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 78ca1218c..c3fb3cfd2 100644
--- 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 8a1c23049..aec6a6eca 100644
--- 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 9d21297b0..d9791431d 100644
--- 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");
 	}