Patches for fixing module load/unload panics (The files)

[Nicolai Petri]

It seems my attachments went to /dev/null on this list so here they are pasted 
in :

********************* Patch 1 **************************
--- kern_linker.c.orig	Mon Mar  3 23:53:35 2003
+++ kern_linker.c	Sat May  3 11:40:09 2003
@@ -290,6 +335,32 @@
 		} else
 			return (0);
 	}
+	/* Run thru all modules and make sure they don't collide */
+	MOD_SLOCK;
+	for (mdp = start; mdp < stop; mdp++) {
+		if ((*mdp)->md_type != MDT_MODULE)
+			continue;
+		moddata = (*mdp)->md_data;
+		KLD_DPF(FILE, ("Registering module %s in %s\n",
+		    moddata->name, lf->filename));
+		if (module_lookupbyname(moddata->name)) 
+		{
+			if (lf != linker_kernel_file)
+			{
+				printf("linker_file_register_modules:"
+				    " Cannot load module %s.\n" 
+				    "linker_file_register_modules: "
+				    "Module %s is already loaded.\n",
+				    lf->filename, moddata->name);
+				MOD_SUNLOCK;
+				return EEXIST;
+			} else {
+				printf("linker_file_register_modules: "
+				    " Conflict! Forcing kernel load anyway!\n");
+			}
+		}
+	}
+	MOD_SUNLOCK;
 	for (mdp = start; mdp < stop; mdp++) {
 		if ((*mdp)->md_type != MDT_MODULE)
 			continue;
@@ -351,7 +422,14 @@
 		if (error != ENOENT)
 			foundfile = 1;
 		if (lf) {
-			linker_file_register_modules(lf);
+			if (linker_file_register_modules(lf))
+			{
+				printf("linker_load_file: "
+				    "register_modules_failed\n");
+				linker_file_unload(lf);
+				error = EEXIST;
+				goto out;

********************* Patch 2 **************************
--- kern_sysctl.c.orig	Fri May  2 21:19:10 2003
+++ kern_sysctl.c	Sat May  3 12:13:07 2003
@@ -153,7 +153,30 @@
 void
 sysctl_unregister_oid(struct sysctl_oid *oidp)
 {
-	SLIST_REMOVE(oidp->oid_parent, oidp, sysctl_oid, oid_link);
+	struct sysctl_oid *p;
+	int error = ENOENT;
+	
+	if (oidp->oid_number == OID_AUTO) 
+		error=EINVAL;
+	else SLIST_FOREACH(p, oidp->oid_parent, oid_link) {
+		if (p == oidp)
+		{
+			SLIST_REMOVE(oidp->oid_parent, oidp, 
+			    sysctl_oid, oid_link);
+			error=0;
+			break;
+		}
+        }
+	/* This can happen when a module fails to register and 
+	 * is being unloaded afterwards. 
+	 * It should not be a panic() for normal use. 
+	 */
+	if (error)
+		printf("Sysctl unregister failed\n");
 }
 
 /* Initialize a new context to keep track of dynamically added sysctls. */

********************* END **************************


On Sunday 04 May 2003 11:05, Nicolai Petri wrote:
> Hi all
>
> As the codefreeze is due very soon now (tm) I send in theese patches and
> hopes theese get commited before.
>
> patch 1 (kern_linker.patch) :
> 	This fixes panics related to loading a module that's already compiled
> 	into the kernel. (patch 2 avoids some of the problems too)
> 	It works by traversing the loaded module list, and if it's already loaded
> 	it simply aborts the moduleregistering process and returns EEXISTS.
>
> patch 2 (kern_sysctl.patch) :
> 	This patch is somewhat related to the same issue as patch 1. When
> 	loading a	module that contains a sysctl already present in the kernel,
> 	the sysctl_register fails. When unloading the module again it calls
> 	sysctl_unregister() and that leads to instant panic because of missing
> 	validity checks in sysctl_unregister.
>
> The above 2 patches fixes 70-80 % of the kld unload related crashes I've
> seen so far, but I know there is many ghosts hidden in the current
> kld-framework and I'm trying to tidy this up a bit. Being realistic this
> means that a lot of patches like the above will go to the 5.x tree and
> hopefully a reworked kld-framework for 6.x.
>
> In my quest for fixing/redesigning the kld framework I'm currently working
> on a document describing my thoughts on this matter.
> The document and newest versions of my patches can be found on this
> website : http://hobbes.bsd-dk.dk/~npp/
>
> Cheers,
> Nicolai Petri
> catpipe Systems ApS