Skip to content
Snippets Groups Projects
Commit 80bf0ee9 authored by Corey Farrell's avatar Corey Farrell
Browse files

loader: Minor fix to module registration.

This protects the module loader itself against crashing if dlopen is
called on a module from outside loader.c.

* Expand scope of lock inside ast_module_register to include reading of
  resource_being_loaded.
* NULL check resource_being_loaded.
* Set resource_being_loaded NULL as soon as dlopen returns.  This fixes
  some error paths where it was not NULL'ed.
* Create module_destroy function to deduplicate code from
  ast_module_unregister and modules_shutdown.
* Resolve leak that occured if a module did not successfully register.
* Simplify checking for successful registration.

Change-Id: I40f07a315e55b92df4fc7faf525ed6d4f396e7d2
parent 30658e71
Branches
Tags
No related merge requests found
......@@ -167,38 +167,51 @@ static int do_full_reload = 0;
static AST_DLLIST_HEAD_STATIC(reload_queue, reload_queue_item);
/* when dynamic modules are being loaded, ast_module_register() will
need to know what filename the module was loaded from while it
is being registered
*/
/*!
* \internal
*
* This variable is set by load_dynamic_module so ast_module_register
* can know what pointer is being registered.
*
* This is protected by the module_list lock.
*/
static struct ast_module *resource_being_loaded;
/* XXX: should we check for duplicate resource names here? */
/*!
* \internal
* \brief Used by AST_MODULE_INFO to register with the module loader.
*
* This function is automatically called when each module is opened.
* It must never be used from outside AST_MODULE_INFO.
*/
void ast_module_register(const struct ast_module_info *info)
{
struct ast_module *mod = resource_being_loaded;
struct ast_module *mod;
/*
* This lock protects resource_being_loaded as well as the module
* list. Normally we already have a lock on module_list when we
* begin the load but locking again from here prevents corruption
* if an asterisk module is dlopen'ed from outside the module loader.
*/
AST_DLLIST_LOCK(&module_list);
mod = resource_being_loaded;
if (!mod) {
AST_DLLIST_UNLOCK(&module_list);
return;
}
ast_debug(5, "Registering module %s\n", info->name);
/* This tells load_dynamic_module that we're registered. */
resource_being_loaded = NULL;
mod->info = info;
if (ast_opt_ref_debug) {
mod->ref_debug = ao2_t_alloc(0, NULL, info->name);
}
AST_LIST_HEAD_INIT(&mod->users);
/* during startup, before the loader has been initialized,
there are no threads, so there is no need to take the lock
on this list to manipulate it. it is also possible that it
might be unsafe to use the list lock at that point... so
let's avoid it altogether
*/
AST_DLLIST_LOCK(&module_list);
/* it is paramount that the new entry be placed at the tail of
the list, otherwise the code that uses dlopen() to load
dynamic modules won't be able to find out if the module it
just opened was registered or failed to load
*/
AST_DLLIST_INSERT_TAIL(&module_list, mod, entry);
AST_DLLIST_UNLOCK(&module_list);
......@@ -206,6 +219,13 @@ void ast_module_register(const struct ast_module_info *info)
*((struct ast_module **) &(info->self)) = mod;
}
static void module_destroy(struct ast_module *mod)
{
AST_LIST_HEAD_DESTROY(&mod->users);
ao2_cleanup(mod->ref_debug);
ast_free(mod);
}
void ast_module_unregister(const struct ast_module_info *info)
{
struct ast_module *mod = NULL;
......@@ -226,9 +246,7 @@ void ast_module_unregister(const struct ast_module_info *info)
if (mod) {
ast_debug(5, "Unregistering module %s\n", info->name);
AST_LIST_HEAD_DESTROY(&mod->users);
ao2_cleanup(mod->ref_debug);
ast_free(mod);
module_destroy(mod);
}
}
......@@ -511,6 +529,8 @@ static struct ast_module *load_dynamic_module(const char *resource_in, unsigned
int space; /* room needed for the descriptor */
int missing_so = 0;
ast_assert(!resource_being_loaded);
space = sizeof(*resource_being_loaded) + strlen(resource_in) + 1;
if (strcasecmp(resource_in + strlen(resource_in) - 3, ".so")) {
missing_so = 1;
......@@ -523,34 +543,23 @@ static struct ast_module *load_dynamic_module(const char *resource_in, unsigned
any symbols, and don't export any symbols. this will allow us to peek into
the module's info block (if available) to see what flags it has set */
resource_being_loaded = ast_calloc(1, space);
mod = resource_being_loaded = ast_calloc(1, space);
if (!resource_being_loaded)
return NULL;
strcpy(resource_being_loaded->resource, resource_in);
if (missing_so)
strcat(resource_being_loaded->resource, ".so");
if (!(lib = dlopen(fn, RTLD_LAZY | RTLD_GLOBAL))) {
if (!suppress_logging) {
lib = dlopen(fn, RTLD_LAZY | RTLD_GLOBAL);
if (resource_being_loaded) {
resource_being_loaded = NULL;
if (lib) {
ast_log(LOG_ERROR, "Module '%s' did not register itself during load\n", resource_in);
logged_dlclose(resource_in, lib);
} else if (!suppress_logging) {
ast_log(LOG_WARNING, "Error loading module '%s': %s\n", resource_in, dlerror());
}
ast_free(resource_being_loaded);
return NULL;
}
/* the dlopen() succeeded, let's find out if the module
registered itself */
/* note that this will only work properly as long as
ast_module_register() (which is called by the module's
constructor) places the new module at the tail of the
module_list
*/
if (resource_being_loaded != (mod = AST_DLLIST_LAST(&module_list))) {
ast_log(LOG_WARNING, "Module '%s' did not register itself during load\n", resource_in);
/* no, it did not, so close it and return */
logged_dlclose(resource_in, lib);
/* note that the module's destructor will call ast_module_unregister(),
which will free the structure we allocated in resource_being_loaded */
ast_free(mod);
return NULL;
}
......@@ -564,19 +573,20 @@ static struct ast_module *load_dynamic_module(const char *resource_in, unsigned
}
logged_dlclose(resource_in, lib);
resource_being_loaded = NULL;
/* start the load process again */
resource_being_loaded = ast_calloc(1, space);
mod = resource_being_loaded = ast_calloc(1, space);
if (!resource_being_loaded)
return NULL;
strcpy(resource_being_loaded->resource, resource_in);
if (missing_so)
strcat(resource_being_loaded->resource, ".so");
if (!(lib = dlopen(fn, wants_global ? RTLD_LAZY | RTLD_GLOBAL : RTLD_NOW | RTLD_LOCAL))) {
lib = dlopen(fn, wants_global ? RTLD_LAZY | RTLD_GLOBAL : RTLD_NOW | RTLD_LOCAL);
resource_being_loaded = NULL;
if (!lib) {
ast_log(LOG_WARNING, "Error loading module '%s': %s\n", resource_in, dlerror());
ast_free(resource_being_loaded);
ast_free(mod);
return NULL;
}
......@@ -585,7 +595,6 @@ static struct ast_module *load_dynamic_module(const char *resource_in, unsigned
time too :) */
AST_DLLIST_LAST(&module_list)->lib = lib;
resource_being_loaded = NULL;
return AST_DLLIST_LAST(&module_list);
}
......@@ -621,9 +630,7 @@ int modules_shutdown(void)
ast_verb(1, "Unloading %s\n", mod->resource);
mod->info->unload();
}
AST_LIST_HEAD_DESTROY(&mod->users);
ao2_cleanup(mod->ref_debug);
ast_free(mod);
module_destroy(mod);
somethingchanged = 1;
}
AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_END;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment