From: Marek Vavruša <marek.vavrusa@nic.cz>
Date: Sun, 24 May 2015 21:29:01 +0000 (+0200)
Subject: modules: fixed module unload touching freed memory
X-Git-Tag: v1.0.0-beta1~139
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=efccea8e960e704d8c3271211700c416c7e47604;p=thirdparty%2Fknot-resolver.git

modules: fixed module unload touching freed memory

the module memory not be moved (reallocd), turned array of modules to array of pointers to modules
---

diff --git a/daemon/bindings.c b/daemon/bindings.c
index adc58c48f..f69f01675 100644
--- a/daemon/bindings.c
+++ b/daemon/bindings.c
@@ -59,7 +59,7 @@ static int mod_list(lua_State *L)
 	struct engine *engine = engine_luaget(L);
 	lua_newtable(L);
 	for (unsigned i = 0; i < engine->modules.len; ++i) {
-		struct kr_module *module = &engine->modules.at[i];
+		struct kr_module *module = engine->modules.at[i];
 		lua_pushstring(L, module->name);
 		lua_rawseti(L, -2, i + 1);
 	}
diff --git a/daemon/engine.c b/daemon/engine.c
index 887cc36b9..e37648456 100644
--- a/daemon/engine.c
+++ b/daemon/engine.c
@@ -212,6 +212,19 @@ int engine_init(struct engine *engine, mm_ctx_t *pool)
 	return ret;
 }
 
+static void engine_unload(struct engine *engine, struct kr_module *module)
+{
+	/* Unregister module */
+	auto_free char *name = strdup(module->name);
+	kr_module_unload(module);
+	/* Clear in Lua world */
+	if (name) {
+		lua_pushnil(engine->L);
+		lua_setglobal(engine->L, name);
+	}
+	free(module);
+}
+
 void engine_deinit(struct engine *engine)
 {
 	if (engine == NULL) {
@@ -222,7 +235,7 @@ void engine_deinit(struct engine *engine)
 
 	/* Unload modules. */
 	for (size_t i = 0; i < engine->modules.len; ++i) {
-		kr_module_unload(&engine->modules.at[i]);
+		engine_unload(engine, engine->modules.at[i]);
 	}
 	array_clear(engine->modules);
 	array_clear(engine->storage_registry);
@@ -353,18 +366,23 @@ int engine_register(struct engine *engine, const char *name)
 	/* Make sure module is unloaded */
 	(void) engine_unregister(engine, name);
 	/* Attempt to load binary module */
-	size_t next = engine->modules.len;
-	array_reserve(engine->modules, next + 1);
-	struct kr_module *module = &engine->modules.at[next];
+	struct kr_module *module = malloc(sizeof(*module));
+	if (!module) {
+		return kr_error(ENOMEM);
+	}
 	int ret = kr_module_load(module, name, NULL);
 	/* Load Lua module if not a binary */
 	if (ret == kr_error(ENOENT)) {
 		ret = ffimodule_register_lua(engine, module, name);
 	}
 	if (ret != 0) {
+		free(module);
 		return ret;
-	} else {
-		engine->modules.len += 1;
+	}
+
+	if (array_push(engine->modules, module) < 0) {
+		engine_unload(engine, module);
+		return kr_error(ENOMEM);
 	}
 
 	/* Register properties */
@@ -381,18 +399,15 @@ int engine_unregister(struct engine *engine, const char *name)
 	module_array_t *mod_list = &engine->modules;
 	size_t found = mod_list->len;
 	for (size_t i = 0; i < mod_list->len; ++i) {
-		if (strcmp(mod_list->at[i].name, name) == 0) {
+		struct kr_module *mod = mod_list->at[i];
+		if (strcmp(mod->name, name) == 0) {
 			found = i;
 			break;
 		}
 	}
-
-	/* Unregister module */
 	if (found < mod_list->len) {
-		kr_module_unload(&mod_list->at[found]);
+		engine_unload(engine, mod_list->at[found]);
 		array_del(*mod_list, found);
-		lua_pushnil(engine->L);
-		lua_setglobal(engine->L, name);
 		return kr_ok();
 	}
 
diff --git a/daemon/ffimodule.c b/daemon/ffimodule.c
index 43349e1bf..5232dbb91 100644
--- a/daemon/ffimodule.c
+++ b/daemon/ffimodule.c
@@ -70,6 +70,7 @@ static inline int l_ffi_call(lua_State *L, int argc)
 {
 	int status = lua_pcall(L, argc, LUA_MULTRET, 0);
 	if (status != 0) {
+		fprintf(stderr, "error: %s\n", lua_tostring(L, -1));
 		lua_pop(L, 1);
 		return kr_error(EIO);
 	}
@@ -110,8 +111,6 @@ static int l_ffi_deinit(struct kr_module *module)
 	/* Unref module and unset 'lib', so the module
 	 * interface doesn't attempt to close it.
 	 */
-	lua_pushnil(L);
-	lua_setglobal(L, module->name);
 	luaL_unref(L, LUA_REGISTRYINDEX, (intptr_t)module->data);
 	module->lib = NULL;
 	return ret;
diff --git a/lib/module.c b/lib/module.c
index e73db8809..19c93bc97 100644
--- a/lib/module.c
+++ b/lib/module.c
@@ -191,8 +191,6 @@ void kr_module_unload(struct kr_module *module)
 		return;
 	}
 
-	free(module->name);
-
 	if (module->deinit) {
 		module->deinit(module);
 	}
@@ -201,5 +199,6 @@ void kr_module_unload(struct kr_module *module)
 		dlclose(module->lib);
 	}
 
+	free(module->name);
 	memset(module, 0, sizeof(struct kr_module));
 }
diff --git a/lib/resolve.c b/lib/resolve.c
index 29237e5e7..9f0425381 100644
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -69,7 +69,7 @@ static void prepare_layers(struct kr_request *param)
 {
 	struct kr_context *ctx = param->ctx;
 	for (size_t i = 0; i < ctx->modules->len; ++i) {
-		struct kr_module *mod = &ctx->modules->at[i];
+		struct kr_module *mod = ctx->modules->at[i];
 		if (mod->layer) {
 			knot_overlay_add(&param->overlay, mod->layer(mod), param);
 		}
diff --git a/lib/resolve.h b/lib/resolve.h
index eefc75d0d..70f72de6b 100644
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -84,7 +84,7 @@
  */
 
 /* @cond internal Array of modules. */
-typedef array_t(struct kr_module) module_array_t;
+typedef array_t(struct kr_module *) module_array_t;
 /* @endcond */
 
 /**