[icinga-checkins] icinga.org: icinga-core/next: nebmods: fix multiple modules sharing same symbols not receiving callback data

git at icinga.org git at icinga.org
Wed May 22 13:27:53 CEST 2013


Module: icinga-core
Branch: next
Commit: 524838659e715f1edc57f0ef96d8ab02544347ef
URL:    https://git.icinga.org/?p=icinga-core.git;a=commit;h=524838659e715f1edc57f0ef96d8ab02544347ef

Author: Michael Friedrich <michael.friedrich at netways.de>
Date:   Sat May 18 20:51:17 2013 +0200

nebmods: fix multiple modules sharing same symbols not receiving callback data

once dlopen() loads modules with the same name, it will make sure to
register only the first symbols, and not overriding the coming ones from
more modules of the same binary.

this is bad, as e.g. multiple idomod's will try to register the same
callback function symbols, and only the first one wins then. this is the
reason why only the first instance_name is getting populated in idoutils
database then too.

the initial handshake of ido2db with idomod only happens for the reason
of calling the nebmodule_init() function symbol (from the first module)
with different arguments, passed by the core. each module should have at
least a different instance_name defined in its idomod.cfg, so the init
function will be called with those, doing a handshake (and insert into
icinga_instances table then) then. but afterwards, no data gets received
from the callbacks not having such a diversity (how should the core then
know anyways, it's a _callback_ function called only once from symbol
space!).

basically, the root issue is an omd patch living for more than 2 years
now, but the old behaviour is now restored with this fix (plus adding a
debug idea stolen from Andreas Ericsson, thanks).
alongside fixing this, the solution is rather simple - make a temp copy
of the binary to some random name, and let dlopen actually load that
into memory. then all the registered function symbols will remain module
based and everything else works with the callbacks and so on.

as this is a behavioural change, it should get a note in Changelog once
merged into release trees.

refs #4199

---

 base/nebmods.c       |   52 +++++++++++++++++++++++++++++++++++++++----------
 include/nebmodules.h |    1 +
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/base/nebmods.c b/base/nebmods.c
index 3395a54..42df25d 100644
--- a/base/nebmods.c
+++ b/base/nebmods.c
@@ -37,6 +37,7 @@ nebmodule *neb_module_list = NULL;
 nebcallback **neb_callback_list = NULL;
 
 extern char     *temp_path;
+extern int	daemon_dumps_core;
 
 
 /* compat stuff for USE_LTDL */
@@ -173,8 +174,8 @@ int neb_load_all_modules(void) {
 int neb_load_module(nebmodule *mod) {
 	int (*initfunc)(int, char *, void *);
 	int *module_version_ptr = NULL;
-	int result = OK;
-
+	int dest_fd, result = OK;
+	char output_file[PATH_MAX];
 
 	if (mod == NULL || mod->filename == NULL)
 		return ERROR;
@@ -201,14 +202,24 @@ int neb_load_module(nebmodule *mod) {
 	   So... the trick is to (1) copy the module to a temp file, (2) dlopen() the temp file, and (3) immediately delete the temp file.
 	************/
 
-	/* 2010-01-05 MF: Patch taken from OMD into Icinga Core
-		   OMD: Do not make a copy of the module, but directly load it. This prevents problems with a tmpfs which
-	   is mounted as user. OMD users surely have no problems with modules overwritten by 'cp in runtime. Anyway,
-	   the usual way to install files is 'install', which removes and recreates the file (just as tar, rpm and
-	   many other installation-tools do). */
+	/*
+	 * create a temp copy with a different name,
+	 * not sharing the global symbol space with
+	 * other neb modules of the same binary.
+	 * check https://dev.icinga.org/issues/4199
+	 */
+	snprintf(output_file, sizeof(output_file)-1, "%s/icinganebmodXXXXXX", temp_path);
+	dest_fd = mkstemp(output_file);
+	result = my_fdcopy(mod->filename, output_file, dest_fd);
+	close(dest_fd);
+
+	if (result == ERROR) {
+		logit(NSLOG_RUNTIME_ERROR, TRUE, "Error: Failed to safely copy module '%s' to '%s'. The module will not be loaded\n", mod->filename, output_file);
+		return ERROR;
+	}
 
 	/* load the module (use the temp copy we just made) */
-	mod->module_handle = dlopen(mod->filename, RTLD_NOW | RTLD_GLOBAL);
+	mod->module_handle = dlopen(output_file, RTLD_NOW | RTLD_GLOBAL);
 
 	if (mod->module_handle == NULL) {
 
@@ -217,6 +228,22 @@ int neb_load_module(nebmodule *mod) {
 		return ERROR;
 	}
 
+	/* mark the module as being loaded */
+	mod->is_currently_loaded = TRUE;
+
+	/* delete the temp copy of the module just loaded */
+	if (unlink(output_file) == -1) {
+		logit(NSLOG_RUNTIME_ERROR, TRUE, "Error: Could not delete temporary file '%s' used for module '%s'. The module will be unloaded: %s", output_file, mod->filename, strerror(errno));
+		neb_unload_module(mod, NEBMODULE_FORCE_UNLOAD, NEBMODULE_ERROR_API_VERSION);
+	}
+
+	/* if we're debugging, create a copy for debuggers finding the correct symbols */
+	if (daemon_dumps_core == TRUE) {
+		dest_fd = open(output_file, O_CREAT | O_WRONLY, S_IRWXU | S_IRGRP | S_IROTH);
+		result = my_fdcopy(mod->filename, output_file, dest_fd);
+		mod->dl_file = strdup(output_file);
+	}
+
 	/* add a compatibility check for 1.7 change of idomod.o -> idomod.so */
 	/* FIXME - drop in 1.8 */
 	if (strstr(mod->filename, "idomod.o") != NULL) {
@@ -229,9 +256,6 @@ int neb_load_module(nebmodule *mod) {
 	/* find module API version */
 	module_version_ptr = (int *)dlsym(mod->module_handle, "__neb_api_version");
 
-	/* mark the module as being loaded */
-	mod->is_currently_loaded = TRUE;
-
 	/* check the module API version */
 	if (module_version_ptr == NULL || ((*module_version_ptr) != CURRENT_NEB_API_VERSION)) {
 
@@ -332,6 +356,12 @@ int neb_unload_module(nebmodule *mod, int flags, int reason) {
 
 	log_debug_info(DEBUGL_EVENTBROKER, 0, "Attempting to unload module '%s': flags=%d, reason=%d\n", mod->filename, flags, reason);
 
+	/* if we're debugging, remove the copied file */
+	if (daemon_dumps_core == TRUE && mod->dl_file) {
+		result = unlink(mod->dl_file);
+		my_free(mod->dl_file);
+	}
+
 	/* call the de-initialization function if available (and the module was initialized) */
 	if (mod->deinit_func && reason != NEBMODULE_ERROR_BAD_INIT) {
 
diff --git a/include/nebmodules.h b/include/nebmodules.h
index 63cd373..13b2455 100644
--- a/include/nebmodules.h
+++ b/include/nebmodules.h
@@ -90,6 +90,7 @@ typedef struct nebmodule_struct{
 	pthread_t       thread_id;
 #endif
 	struct nebmodule_struct *next;
+	char		*dl_file;
         }nebmodule;
 
 





More information about the icinga-checkins mailing list