From: George Joseph <george.joseph@fairview5.com>
Date: Thu, 9 Apr 2015 22:35:11 +0000 (+0000)
Subject: res_pjsip_phoneprov_provider: Fix reference leak on unload
X-Git-Tag: 13.4.0-rc1~134
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=75c2c859620edc9bfc03fada26bb83e90013b33e;p=thirdparty%2Fasterisk.git

res_pjsip_phoneprov_provider: Fix reference leak on unload

res_pjsip_phoneprov_provider was leaking references to phoneprov objects due to
a missing OBJ_NODATA in an ao2_callback in load_users().  Rather than adding the
OBJ_NODATA, I changed load_users to use a more straightforward ao2_iterator.
This plugged the leak but exposed an unload order issue between
res_pjsip_phoneprov_provider, res_phoneprov and res_pjsip.

res_pjsip_phoneprov_provider unloads first, then res_phoneprov, then res_pjsip.
Since res_pjsip_phoneprov_provider uses res_pjsip's sorcery instance, when it
unloads, it's objects are still in the sorcery instance.  When res_pjsip
unloads, it destroys all its objects including res_pjsip_phoneprov_provider's.
The phoneprov destructor then attempts to unregister the extension from
res_phoneprov but because res_phoneprov is already cleaned up, its users
container is gone and we get a FRACK.

Simple solution, check for the NULL users container before attempting to remove
the entry. Duh.

Ran tests/res_phoneprov/res_phoneprov_provider.  No leaks in
res_pjsip_phoneprov_provider and no FRACKs.

Reported-by: Corey Farrell
Tested-by: George Joseph
Review: https://reviewboard.asterisk.org/r/4608/
ASTERISK-24935 #close



git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/13@434545 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---

diff --git a/res/res_phoneprov.c b/res/res_phoneprov.c
index ce008d20fe..6986d203ff 100644
--- a/res/res_phoneprov.c
+++ b/res/res_phoneprov.c
@@ -1581,12 +1581,20 @@ static int extension_delete_cb(void *obj, void *arg, void *data, int flags)
 
 void ast_phoneprov_delete_extension(char *provider_name, char *macaddress)
 {
+	if (!users) {
+		return;
+	}
+
 	ao2_callback_data(users, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE | OBJ_SEARCH_KEY,
 		extension_delete_cb, macaddress, provider_name);
 }
 
 void ast_phoneprov_delete_extensions(char *provider_name)
 {
+	if (!users) {
+		return;
+	}
+
 	ao2_callback(users, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, extensions_delete_cb, provider_name);
 }
 
diff --git a/res/res_pjsip_phoneprov_provider.c b/res/res_pjsip_phoneprov_provider.c
index 70dd99e067..8dafabc53d 100644
--- a/res/res_pjsip_phoneprov_provider.c
+++ b/res/res_pjsip_phoneprov_provider.c
@@ -286,9 +286,8 @@ static int load_endpoint(const char *id, const char *endpoint_name, struct varsh
 }
 
 /*! \brief Callback that validates the phoneprov object */
-static int users_apply_handler(void *obj, void *arg, int flags)
+static void users_apply_handler(struct phoneprov *pp)
 {
-	struct phoneprov *pp = obj;
 	const char *id = ast_sorcery_object_get_id(pp);
 	const char *endpoint_name;
 	char port_string[6];
@@ -296,19 +295,19 @@ static int users_apply_handler(void *obj, void *arg, int flags)
 	if (!ast_var_find(pp->vars,
 		ast_phoneprov_std_variable_lookup(AST_PHONEPROV_STD_MAC))) {
 		ast_log(LOG_ERROR, "phoneprov %s must contain a MAC entry.\n", id);
-		return 0;
+		return;
 	}
 
 	if (!ast_var_find(pp->vars,
 		ast_phoneprov_std_variable_lookup(AST_PHONEPROV_STD_PROFILE))) {
 		ast_log(LOG_ERROR, "phoneprov %s must contain a PROFILE entry.\n", id);
-		return 0;
+		return;
 	}
 
 	endpoint_name = ast_var_find(pp->vars, "endpoint");
 	if (endpoint_name) {
 		if (load_endpoint(id, endpoint_name, pp->vars, port_string)) {
-			return 0;
+			return;
 		}
 	}
 
@@ -337,13 +336,15 @@ static int users_apply_handler(void *obj, void *arg, int flags)
 
 	ast_phoneprov_add_extension(AST_MODULE, pp->vars);
 
-	return CMP_MATCH;
+	return;
 }
 
 /*! \brief Callback that loads the users from phoneprov sections */
 static int load_users(void)
 {
 	struct ao2_container *users;
+	struct ao2_iterator i;
+	struct phoneprov *pp;
 
 	ast_sorcery_reload_object(sorcery, "phoneprov");
 
@@ -353,13 +354,17 @@ static int load_users(void)
 		return 0;
 	}
 
-	ao2_callback(users, OBJ_MULTIPLE, users_apply_handler, sorcery);
+	i = ao2_iterator_init(users, 0);
+	while ((pp = ao2_iterator_next(&i))) {
+		users_apply_handler(pp);
+		ao2_ref(pp, -1);
+	}
+	ao2_iterator_destroy(&i);
 	ao2_ref(users, -1);
 
 	return 0;
 }
 
-
 static int load_module(void)
 {
 	CHECK_PJSIP_MODULE_LOADED();