[Asterisk-code-review] res_pjsip_pubsub: Prevent removing subscriptions. (asterisk[20])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19486 )

Change subject: res_pjsip_pubsub: Prevent removing subscriptions.
......................................................................

res_pjsip_pubsub: Prevent removing subscriptions.

pjproject does not provide any mechanism of removing
event packages, which means that once a subscription
handler is registered, it is effectively permanent.

pjproject will assert if the same event package is
ever registered again, so currently unloading and
loading any Asterisk modules that use subscriptions
will cause a crash that is beyond our control.

For that reason, we now prevent users from being
able to unload these modules, to prevent them
from ever being loaded twice.

ASTERISK-30264 #close

Change-Id: I7fdcb1a5e44d38b7ba10c44259fe98f0ae9bc12c
---
M res/res_pjsip_exten_state.c
M res/res_pjsip_mwi.c
2 files changed, 50 insertions(+), 0 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/res/res_pjsip_exten_state.c b/res/res_pjsip_exten_state.c
index 3c7baee..1b29091 100644
--- a/res/res_pjsip_exten_state.c
+++ b/res/res_pjsip_exten_state.c
@@ -973,6 +973,7 @@
 
 static int unload_module(void)
 {
+#if 0
 	ast_sip_unregister_event_publisher_handler(&dialog_publisher);
 	ast_sip_unregister_subscription_handler(&dialog_handler);
 	ast_sip_unregister_event_publisher_handler(&presence_publisher);
@@ -987,6 +988,18 @@
 	publishers = NULL;
 
 	return 0;
+#else
+	/* If we were allowed to unload, the above is what we would do.
+	 * pjsip_evsub_register_pkg is called by ast_sip_register_subscription_handler
+	 * but there is no corresponding unregister function, so unloading
+	 * a module does not remove the event package. If this module is ever
+	 * loaded again, then pjproject will assert and cause a crash.
+	 * For that reason, we must not be allowed to unload, but if
+	 * a pjsip_evsub_unregister_pkg API is added in the future
+	 * then we should go back to unloading the module as intended.
+	 */
+	return -1;
+#endif
 }
 
 static int load_module(void)
diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index ba68486..dfe5481 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -1524,6 +1524,7 @@
 
 static int unload_module(void)
 {
+#if 0
 	struct ao2_container *unsolicited_mwi;
 
 	ast_sorcery_observer_remove(ast_sip_get_sorcery(), "global", &global_observer);
@@ -1548,6 +1549,18 @@
 	ast_free(default_voicemail_extension);
 	default_voicemail_extension = NULL;
 	return 0;
+#else
+	/* If we were allowed to unload, the above is what we would do.
+	 * pjsip_evsub_register_pkg is called by ast_sip_register_subscription_handler
+	 * but there is no corresponding unregister function, so unloading
+	 * a module does not remove the event package. If this module is ever
+	 * loaded again, then pjproject will assert and cause a crash.
+	 * For that reason, we must not be allowed to unload, but if
+	 * a pjsip_evsub_unregister_pkg API is added in the future
+	 * then we should go back to unloading the module as intended.
+	 */
+	return -1;
+#endif
 }
 
 static int load_module(void)

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19486
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 20
Gerrit-Change-Id: I7fdcb1a5e44d38b7ba10c44259fe98f0ae9bc12c
Gerrit-Change-Number: 19486
Gerrit-PatchSet: 2
Gerrit-Owner: N A <asterisk at phreaknet.org>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221026/ee9d8ef7/attachment.html>