Re: modutils-2.3.21: modprobe looping

[Kurt Garloff <garloff@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1049 bytes --]

On Tue, Nov 28, 2000 at 10:24:48PM +0100, Kurt Garloff wrote:
> This is a pity, because I think the current behaviour is not acceptable,
> as it can kill the machine by just being invoked by kmod.
> I will try to make sense out of the code and make sure that modprobe
> will not go crazy, by either detecting loops (if I can do that in a way
> wihtout breaking things) or by limiting the recursion depth. I'll send
> you a patch.

Find it attached. As the dependency generation looked indeed rather
cumbersome to me, I didn't really touch it. I just did implement the
recursion limit to prevent the modprobe process grabbing all the memory of
the system ...
root@cantaloupe:~ > modprobe -k pppoe
modprobe: Too deep recursion in module dependencies!
modprobe: Circular dependency? pppox pppoe
Aborted

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                          Eindhoven, NL
GPG key: See mail header, key servers         Linux kernel development
SuSE GmbH, Nuernberg, FRG                               SCSI, Security

[-- Attachment #1.2: modutils-2.3.21-limitrecursion.diff --]
[-- Type: text/plain, Size: 1911 bytes --]

diff -uNr modutils-2.3.21/insmod/modprobe.c modutils-2.3.21.SuSE/insmod/modprobe.c
--- modutils-2.3.21/insmod/modprobe.c	Tue Nov 21 10:10:54 2000
+++ modutils-2.3.21.SuSE/insmod/modprobe.c	Wed Nov 29 00:39:54 2000
@@ -363,6 +363,9 @@
 	return desc;
 }
 
+/* KG, 2000-11-29: Limit recursion in build_stack() */
+#define BSTACKRECMAX 128
+static int bstackrecurs = 0;
 /*
  * Create a linked list of the stack of modules needed in the kernel
  * in order to satisfy a request for "name"
@@ -395,6 +398,15 @@
 	if (rev && strcmp(rev->kname, desc->kname) == 0)
 		return 0;
 
+	if (++bstackrecurs > BSTACKRECMAX) {
+		fprintf (stderr, "modprobe: Too deep recursion in module dependencies!\n");
+		fprintf (stderr, "modprobe: Circular dependency?");
+		for (lp = *stack; lp; lp = lp->next)
+			fprintf (stderr, " %s", lp->item.d->kname);
+		fprintf (stderr, "\n");
+		abort ();
+	}
+					 
 	/*
 	 * Get the modules that this one should pull in
 	 * according to modules.conf ("above")
@@ -414,7 +426,7 @@
 							rev ? rev : desc,
 							MODE_ABOVE, 1);
 					if (r < 0)
-						return r;
+						{ bstackrecurs--; return r; }
 				}
 			}
 		}
@@ -424,7 +436,7 @@
 		if (strcmp(desc->kname, lp->item.d->kname) == 0) {
 			/* Avoid infinite loops ("above") */
 			if (rev && rev == lp->item.d)
-				return 0;
+				{ bstackrecurs--; return 0; }
 			/* else */
 			/* Remove the entry, it is in the wrong place */
 			if (prev)
@@ -452,7 +464,7 @@
 						dir,
 						rev,
 						MODE_BELOW, 1)) < 0)
-					return r;
+					{ bstackrecurs--; return r; }
 			}
 		}
 	}
@@ -465,10 +477,10 @@
 	for (nod = desc->nod->dep; nod; nod = nod->next) {
 		if ((r = build_stack(stack, nod->key, nod->str, dir, rev,
 			MODE_NORMAL, 0)) < 0)
-			return r;
+			{ bstackrecurs--; return r; }
 	}
 
-	return 0;
+	bstackrecurs--; return 0;
 }
 
 /* For debugging */

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]