Commit a899013d56 for asterisk.org
commit a899013d56808961f4d2666e651760c31f6913f4
Author: George Joseph <gjoseph@sangoma.com>
Date: Tue Apr 21 14:31:56 2026 -0600
pbx_functions: Save module pointer before calling read and write callbacks.
Before ast_func_read and ast_func_write call their respective read and write
callbacks for registered dialplan functions, they use the module pointer in
the registered ast_custom_function structure to increment the module use
count. They then decrement the usecount when the callback returns. This
prevents the providing module from being unloaded while there's a call using
the function.
Some modules, notably func_odbc, create and destroy dialplan functions based
on the contents of a config file. Since the ast_custom_function structure is
dynamically allocated, it could be destroyed on reload which means when the
module's read or write callback returns to the ast_func calls it would try to
decrement the usecount using the module pointer from an ast_custom_function
structure that has already been freed. Proper locking or reference counting
by the module can reduce the possibility of this happening but it can't
prevent it because it doesn't have control after its read or write callback
has returned to ast_func_read or ast_func_write.
To address this, ast_func_read, ast_func_read2 and ast_func_write save the
module pointer to a local variable before calling the module's callback,
then use the saved pointer to decrement the use count. The module
pointer will always be valid if the module is loaded regardless of the
state of the ast_custom_function structure.
Resolves: #1818
diff --git a/main/pbx_functions.c b/main/pbx_functions.c
index 58799449f8..a6df6aaa38 100644
--- a/main/pbx_functions.c
+++ b/main/pbx_functions.c
@@ -618,6 +618,14 @@ int ast_func_read(struct ast_channel *chan, const char *function, char *workspac
struct ast_custom_function *acfptr = ast_custom_function_find(copy);
int res;
struct ast_module_user *u = NULL;
+ /*
+ * The module pointer needs to be saved because some modules, notably func_odbc,
+ * dynamically create and destroy functions so the acfptr might not be valid
+ * when the read callback returns.
+ *
+ * NEVER ASSUME THE acfptr IS STILL VALID AFTER THE CALLBACK RETURNS!
+ */
+ struct ast_module *mod = acfptr ? acfptr->mod : NULL;
if (acfptr == NULL) {
ast_log(LOG_ERROR, "Function %s not registered\n", copy);
@@ -626,24 +634,24 @@ int ast_func_read(struct ast_channel *chan, const char *function, char *workspac
} else if (!is_read_allowed(acfptr)) {
ast_log(LOG_ERROR, "Dangerous function %s read blocked\n", copy);
} else if (acfptr->read) {
- if (acfptr->mod) {
- u = __ast_module_user_add(acfptr->mod, chan);
+ if (mod) {
+ u = __ast_module_user_add(mod, chan);
}
res = acfptr->read(chan, copy, args, workspace, len);
- if (acfptr->mod && u) {
- __ast_module_user_remove(acfptr->mod, u);
+ if (mod && u) {
+ __ast_module_user_remove(mod, u);
}
return res;
} else {
struct ast_str *str = ast_str_create(16);
- if (acfptr->mod) {
- u = __ast_module_user_add(acfptr->mod, chan);
+ if (mod) {
+ u = __ast_module_user_add(mod, chan);
}
res = acfptr->read2(chan, copy, args, &str, 0);
- if (acfptr->mod && u) {
- __ast_module_user_remove(acfptr->mod, u);
+ if (mod && u) {
+ __ast_module_user_remove(mod, u);
}
ast_copy_string(workspace, ast_str_buffer(str), len > ast_str_size(str) ? ast_str_size(str) : len);
ast_free(str);
@@ -661,6 +669,14 @@ int ast_func_read2(struct ast_channel *chan, const char *function, struct ast_st
struct ast_custom_function *acfptr = ast_custom_function_find(copy);
int res;
struct ast_module_user *u = NULL;
+ /*
+ * The module pointer needs to be saved because some modules, notably func_odbc,
+ * dynamically create and destroy functions so the acfptr might not be valid
+ * when the read callback returns.
+ *
+ * NEVER ASSUME THE acfptr IS STILL VALID AFTER THE CALLBACK RETURNS!
+ */
+ struct ast_module *mod = acfptr ? acfptr->mod : NULL;
if (acfptr == NULL) {
ast_log(LOG_ERROR, "Function %s not registered\n", copy);
@@ -669,8 +685,8 @@ int ast_func_read2(struct ast_channel *chan, const char *function, struct ast_st
} else if (!is_read_allowed(acfptr)) {
ast_log(LOG_ERROR, "Dangerous function %s read blocked\n", copy);
} else {
- if (acfptr->mod) {
- u = __ast_module_user_add(acfptr->mod, chan);
+ if (mod) {
+ u = __ast_module_user_add(mod, chan);
}
ast_str_reset(*str);
if (acfptr->read2) {
@@ -695,8 +711,8 @@ int ast_func_read2(struct ast_channel *chan, const char *function, struct ast_st
res = acfptr->read(chan, copy, args, ast_str_buffer(*str), maxsize);
ast_str_update(*str); /* Manually set the string length */
}
- if (acfptr->mod && u) {
- __ast_module_user_remove(acfptr->mod, u);
+ if (mod && u) {
+ __ast_module_user_remove(mod, u);
}
return res;
@@ -710,6 +726,14 @@ int ast_func_write(struct ast_channel *chan, const char *function, const char *v
char *copy = ast_strdupa(function);
char *args = func_args(copy);
struct ast_custom_function *acfptr = ast_custom_function_find(copy);
+ /*
+ * The module pointer needs to be saved because some modules, notably func_odbc,
+ * dynamically create and destroy functions so the acfptr might not be valid
+ * when the write callback returns.
+ *
+ * NEVER ASSUME THE acfptr IS STILL VALID AFTER THE CALLBACK RETURNS!
+ */
+ struct ast_module *mod = acfptr ? acfptr->mod : NULL;
if (acfptr == NULL) {
ast_log(LOG_ERROR, "Function %s not registered\n", copy);
@@ -721,12 +745,12 @@ int ast_func_write(struct ast_channel *chan, const char *function, const char *v
int res;
struct ast_module_user *u = NULL;
- if (acfptr->mod) {
- u = __ast_module_user_add(acfptr->mod, chan);
+ if (mod) {
+ u = __ast_module_user_add(mod, chan);
}
res = acfptr->write(chan, copy, args, value);
- if (acfptr->mod && u) {
- __ast_module_user_remove(acfptr->mod, u);
+ if (mod && u) {
+ __ast_module_user_remove(mod, u);
}
return res;