Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
func: fix use after free on function unload
Functions are stored in lists inside module objects. Module
objects are stored in a hash table, where key is a package name.
But the key was a pointer at one of module's function definition
object. Therefore, when that function was deleted, its freed
package name memory was still in the hash key, and could be
accessed, when another function was deleted.

Now module does not use memory of its functions, and keep a copy
of the package name.

(cherry picked from commit fa2893e)
  • Loading branch information
Gerold103 authored and kyukhin committed Nov 21, 2019
1 parent 84ea1b9 commit 64f4d06
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
25 changes: 14 additions & 11 deletions src/box/func.c
Expand Up @@ -209,12 +209,12 @@ module_cache_find(const char *name, const char *name_end)
* Save module to the module cache.
*/
static inline int
module_cache_put(const char *name, const char *name_end, struct module *module)
module_cache_put(struct module *module)
{
size_t name_len = name_end - name;
uint32_t name_hash = mh_strn_hash(name, name_len);
size_t package_len = strlen(module->package);
uint32_t name_hash = mh_strn_hash(module->package, package_len);
const struct mh_strnptr_node_t strnode = {
name, name_len, name_hash, module};
module->package, package_len, name_hash, module};

if (mh_strnptr_put(modules, &strnode, NULL, NULL) == mh_end(modules)) {
diag_set(OutOfMemory, sizeof(strnode), "malloc", "modules");
Expand Down Expand Up @@ -248,12 +248,16 @@ module_load(const char *package, const char *package_end)
if (module_find(package, package_end, path, sizeof(path)) != 0)
return NULL;

struct module *module = (struct module *) malloc(sizeof(*module));
int package_len = package_end - package;
struct module *module = (struct module *)
malloc(sizeof(*module) + package_len + 1);
if (module == NULL) {
diag_set(OutOfMemory, sizeof(struct module), "malloc",
"struct module");
diag_set(OutOfMemory, sizeof(struct module) + package_len + 1,
"malloc", "struct module");
return NULL;
}
memcpy(module->package, package, package_len);
module->package[package_len] = 0;
rlist_create(&module->funcs);
module->calls = 0;
module->is_unloading = false;
Expand All @@ -264,7 +268,7 @@ module_load(const char *package, const char *package_end)
}
char load_name[PATH_MAX + 1];
snprintf(load_name, sizeof(load_name), "%s/%.*s." TARANTOOL_LIBEXT,
dir_name, (int)(package_end - package), package);
dir_name, package_len, package);
if (symlink(path, load_name) < 0) {
diag_set(SystemError, "failed to create dso link");
goto error;
Expand All @@ -275,7 +279,6 @@ module_load(const char *package, const char *package_end)
if (rmdir(dir_name) != 0)
say_warn("failed to delete temporary dir %s", dir_name);
if (module->handle == NULL) {
int package_len = (int) (package_end - package_end);
diag_set(ClientError, ER_LOAD_MODULE, package_len,
package, dlerror());
goto error;
Expand Down Expand Up @@ -346,7 +349,7 @@ module_reload(const char *package, const char *package_end, struct module **modu
rlist_move(&new_module->funcs, &func->item);
}
module_cache_del(package, package_end);
if (module_cache_put(package, package_end, new_module) != 0)
if (module_cache_put(new_module) != 0)
goto restore;
old_module->is_unloading = true;
module_gc(old_module);
Expand Down Expand Up @@ -515,7 +518,7 @@ func_c_load(struct func_c *func)
module = module_load(name.package, name.package_end);
if (module == NULL)
return -1;
if (module_cache_put(name.package, name.package_end, module)) {
if (module_cache_put(module)) {
module_delete(module);
return -1;
}
Expand Down
2 changes: 2 additions & 0 deletions src/box/func.h
Expand Up @@ -56,6 +56,8 @@ struct module {
size_t calls;
/** True if module is being unloaded. */
bool is_unloading;
/** Module's package name. */
char package[0];
};

/** Virtual method table for func object. */
Expand Down

0 comments on commit 64f4d06

Please sign in to comment.