[syslinux:elflink] elflink: Detect circular dependencies during module load

syslinux-bot for Matt Fleming matt.fleming at linux.intel.com
Mon Apr 4 14:15:11 PDT 2011


Commit-ID:  897ce5596a6f706a97150c4c365b04427b2ec71f
Gitweb:     http://syslinux.zytor.com/commit/897ce5596a6f706a97150c4c365b04427b2ec71f
Author:     Matt Fleming <matt.fleming at linux.intel.com>
AuthorDate: Thu, 31 Mar 2011 13:08:37 +0100
Committer:  Matt Fleming <matt.fleming at linux.intel.com>
CommitDate: Thu, 31 Mar 2011 13:27:26 +0100

elflink: Detect circular dependencies during module load

It's possible for the 'modules.dep' file to contain circular
dependencies where module A has a dependency on module B, and module B
has a depedency on module A.

Currently what happens in the face of circular dependencies when
trying to parse and load the modules in 'modules.dep' is that we get
stuck in a loop in module_load_dependencies(), presumably eventually
blowing the stack.

To fix this we maintain a singly-linked list of module names that are
in the process of having their dependencies loaded, but that have not
completed the loading. Everytime we begin loading the dependencies for
a new module we check to see if that module name is already on the
linked list. If it is, we've discovered a circular reference so we
refuse to load the dependencies and print a helpful message to the
user which includes the circular dependency chain.

It would be possible to cope with these circular references if we were
to skip loading the dependencies for any modules that are on the
linked list and defer resolving any module relocs until after all
module images have been loaded into memory. However, it's more than
likely that a circular reference is indicative of a bug and that the
correct fix would be to extract the symbols causing the circular
dependency into their own module. So we treat circular dependencies as
an error case for now.

Signed-off-by: Matt Fleming <matt.fleming at linux.intel.com>


---
 com32/lib/sys/module/exec.c |   84 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/com32/lib/sys/module/exec.c b/com32/lib/sys/module/exec.c
index 54182cc..fbe165d 100644
--- a/com32/lib/sys/module/exec.c
+++ b/com32/lib/sys/module/exec.c
@@ -343,11 +343,45 @@ int spawn_load(const char *name,const char **argv)
 	*/
 }
 
+/*
+ * Avoid circular dependencies.
+ *
+ * It's possible that someone messed up the modules.dep file and that
+ * it includes circular dependencies, so we need to take steps here to
+ * avoid looping in module_load_dependencies() forever.
+ *
+ * We build a singly-linked list of modules that are in the middle of
+ * being loaded. When they have completed loading their entry is
+ * removed from this list in LIFO order (new entries are always added
+ * to the head of the list).
+ */
+struct loading_dep {
+	const char *name;
+	struct module_dep *next;
+};
+static struct loading_dep *loading_deps;
+
+/*
+ * Remember that because we insert elements in a LIFO order we need to
+ * start from the end of the list and work towards the front so that
+ * we print the modules in the order in which we tried to load them.
+ *
+ * Yay for recursive function calls.
+ */
+static void print_loading_dep(struct loading_dep *dep)
+{
+	if (dep) {
+		print_loading_dep(dep->next);
+		printf("\t\t\"%s\"\n", dep->name);
+	}
+}
+
 int module_load_dependencies(const char *name,const char *dep_file)
 {
 	FILE *d_file=fopen(dep_file,"r");
 	char line[2048],aux[2048],temp_name[MODULE_NAME_SIZE],slbz[24];
 	int i=0,j=0,res=0;
+	struct loading_dep *dep;
 
 	if(d_file==NULL)
 	{
@@ -355,6 +389,40 @@ int module_load_dependencies(const char *name,const char *dep_file)
 		return -1;
 	}
 
+	/*
+	 * Are we already in the middle of loading this module's
+	 * dependencies?
+	 */
+	for (dep = loading_deps; dep; dep = dep->next) {
+		if (!strcasecmp(dep->name, name))
+			break;	/* found */
+	}
+
+	if (dep) {
+		struct loading_dep *last, *prev;
+
+		/* Dup! */
+		printf("\t\tCircular depedency detected when loading "
+		       "modules!\n");
+		printf("\t\tModules dependency chain looks like this,\n\n");
+		print_loading_dep(loading_deps);
+		printf("\n\t\t... and we tried to load \"%s\" again\n", name);
+
+		return -1;
+	} else {
+		dep = malloc(sizeof(*dep));
+		if (!dep) {
+			printf("Failed to alloc memory for loading_dep\n");
+			return -1;
+		}
+		
+		dep->name = name;
+		dep->next = loading_deps;
+
+		/* Insert at the head of the list */
+		loading_deps = dep;
+	}
+
 	/* Note from feng:
 	 * new modues.dep has line like this:
 	 *	a.c32: b.32 c.c32 d.c32
@@ -401,13 +469,21 @@ int module_load_dependencies(const char *name,const char *dep_file)
 
 		if (strlen(temp_name)) {
 			char *argv[2] = { NULL, NULL };
-
-			module_load_dependencies(temp_name, MODULES_DEP);
-			if (spawn_load(temp_name, argv) < 0)
-				continue;
+			int ret;
+
+			ret = module_load_dependencies(temp_name,
+						       MODULES_DEP);
+			if (!ret) {
+				if (spawn_load(temp_name, argv) < 0)
+					continue;
+			}
 		}
 	}
 
+	/* Remove our entry from the head of loading_deps */
+	loading_deps = loading_deps->next;
+	free(dep);
+
 	return 0;
 }
 



More information about the Syslinux-commits mailing list