Bug#524940: module-init-tools: modprobe starts fork-bombing on

[md@Linux.IT (Marco d'Itri)]

[-- Attachment #1: Type: text/plain, Size: 16 bytes --]

-- 
ciao,
Marco

[-- Attachment #2: Type: message/rfc822, Size: 11854 bytes --]

[-- Attachment #2.1.1.1: Type: Text/Plain, Size: 1446 bytes --]

tags 524940 - help
tags 524940 patch
thanks

Hello,

On trečiadienis 05 Rugpjūtis 2009 03:05:06 Marco d'Itri wrote:
> On Aug 05, Eli Mackenzie <eli.mackenzie@gmail.com> wrote:
> > With this bug marked "important", apt-listbugs didn't notify me that
> > there was anything wrong with this package. A failure to boot seems to
> > fit the "makes unrelated software on the system (or the whole system)
> > break" description of "critical" more than "important".
> 
> It only affects a small number of users.
> BTW, the upstream maintainer does not appear to have time to investigate
> this. Feel free to send patches.

The bug can only be reproduced with kernel 2.6.19 or earlier (i.e. the kernel 
in etch). If /sys/module/*/initstate (added in 2.6.20) is not present, current 
modprobe assumes that the module is not loaded. This triggers recursive "fork 
bomb" as modprobe keeps trying to load snd-pcm (i.e. keeps executing that 
custom install command of snd-pcm) again and again. More detailed info about 
this can be found in the patch headers.

To reproduce, either install oss-compat under <= 2.6.19 kernel or reboot to <= 
2.6.19 with oss-compat installed (init=/bin/bash may be helpful to get system 
bootable in this case).

Actually, you need 0002 patch to fix this bug, 0001 is just a safety 
precaution. Please submit both these patches to upstream. Thanks.

-- 
Modestas Vainius <modestas@vainius.eu>

[-- Attachment #2.1.1.2: 0002-Get-module-initstate-from-proc-modules-if-it-is-not-.patch --]
[-- Type: text/x-patch, Size: 3485 bytes --]

From 567f889a3c8d50234abef72b10e3af9312aa808f Mon Sep 17 00:00:00 2001
From: Modestas Vainius <modestas@vainius.eu>
Date: Sun, 20 Sep 2009 14:41:48 +0300
Subject: [PATCH 2/2] Get module initstate from /proc/modules if it is not supported via sysfs.

Get module state from /proc/modules if it is not supported via sysfs.
/sys/module/*/initstate is only available since Linux 2.6.20. Current code
assumes that module is not in the kernel if initstate attribute is absent.
Among other potential problems, this may trigger a "fork bomb" if certain
custom "install" sequences are used for the module and/or its dependencies.
 .
The patch adds a fallback to /proc/modules in order to get the module state if
/sys/module/*/initstate attribute is missing. The code is based on the
module_in_kernel() function from v3.4).

"Fork bombing" side effect of the bug is described in [1]. What's more,
handling of module_in_kernel() failures could still be improved in order avoid
similar problems when /sys is not mounted. Currently, if module_in_kernel()
fails with -1, modprobe assumes the module is not present in the kernel.

1. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=524940

Signed-off-by: Modestas Vainius <modestas@vainius.eu>
---
 modprobe.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/modprobe.c b/modprobe.c
index ec46e0b..06825bd 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -537,6 +537,42 @@ static int read_attribute(const char *filename, char *buf, size_t buflen)
 	return (s == NULL) ? -1 : 1;
 }
 
+/* Since /sys/module/%s/initstate is only available since 2.6.20,
+   fallback to /proc/modules to get module state on earlier kernels.
+   0 means module is not live, 1 means yes, -1 means unknown.
+ */
+static int proc_is_module_live(const char *modname)
+{
+	FILE *proc_modules;
+	char *line;
+	int i, ret;
+
+	/* Might not be mounted yet.  Don't fail. */
+	proc_modules = fopen("/proc/modules", "r");
+	if (!proc_modules)
+		return -1;
+
+	while ((line = getline_wrapped(proc_modules, NULL)) != NULL) {
+		char *entry = strtok(line, " \n");
+
+		if (entry && streq(entry, modname)) {
+			/* If it exists, initstate is the fifth entry. */
+            for (i = 1; i < 5; i++) {
+				if (!(entry = strtok(NULL, " \n")))
+					break;
+			}
+			ret = !(entry && (streq(entry, "Loading") || streq(entry, "Unloading")));
+
+			free(line);
+			fclose(proc_modules);
+			return ret;
+		}
+		free(line);
+	}
+	fclose(proc_modules);
+	return 0;
+}
+
 /* Is module in /sys/module?  If so, fill in usecount if not NULL. 
    0 means no, 1 means yes, -1 means unknown.
  */
@@ -562,10 +598,20 @@ static int module_in_kernel(const char *modname, unsigned int *usecount)
 
 	/* Wait for the existing module to either go live or disappear. */
 	nofail_asprintf(&name, "/sys/module/%s/initstate", modname);
+	ret = 1;
 	while (1) {
-		ret = read_attribute(name, attr, ATTR_LEN);
-		if (ret != 1 || streq(attr, "live\n"))
-			break;
+		/* If ret == 0 upon second and subsequent cycles,
+		   /sys/module/%s/initstate is not supported. Do not try again. */
+		if (ret != 0)
+			ret = read_attribute(name, attr, ATTR_LEN);
+		if (ret == 1) {
+			if (streq(attr, "live\n"))
+				break;
+		} else {
+			ret = proc_is_module_live(modname);
+			if (ret != 0)
+				break;
+		}
 
 		usleep(100000);
 	}
-- 
1.6.4.3


[-- Attachment #2.1.1.3: 0001-Ignore-custom-install-commands-if-failed-to-find-whe.patch --]
[-- Type: text/x-patch, Size: 2073 bytes --]

From 47a0c951d41f586f4e8dfeef3e770449a0fb2570 Mon Sep 17 00:00:00 2001
From: Modestas Vainius <modestas@vainius.eu>
Date: Sun, 20 Sep 2009 16:15:44 +0300
Subject: [PATCH 1/2] Ignore custom install commands if failed to find whether module is loaded.

This should protect from disastrous consequences like in [1] if for some
reason module_in_kernel() status cannot be determined.

1. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=524940

Signed-off-by: Modestas Vainius <modestas@vainius.eu>
---
 modprobe.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/modprobe.c b/modprobe.c
index 21a3111..ec46e0b 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -1095,6 +1095,7 @@ static int insmod(struct list_head *list,
 	const char *command;
 	struct module *mod = list_entry(list->next, struct module, list);
 	int rc = 0;
+	int isloaded = 0;
 
 	/* Take us off the list. */
 	list_del(&mod->list);
@@ -1122,7 +1123,7 @@ static int insmod(struct list_head *list,
 
 	/* Don't do ANYTHING if already in kernel. */
 	if (!(flags & mit_ignore_loaded)
-	    && module_in_kernel(newname ?: mod->modname, NULL) == 1) {
+	    && (isloaded = module_in_kernel(newname ?: mod->modname, NULL) == 1)) {
 		if (flags & mit_first_time)
 			error("Module %s already in kernel.\n",
 			      newname ?: mod->modname);
@@ -1131,10 +1132,15 @@ static int insmod(struct list_head *list,
 
 	command = find_command(mod->modname, commands);
 	if (command && !(flags & mit_ignore_commands)) {
-		close_file(fd);
-		do_command(mod->modname, command, flags & mit_dry_run, error,
-			   "install", cmdline_opts);
-		goto out_optstring;
+		if (isloaded == -1) {
+			error("Unable to find if module %s is loaded. Assuming --ignore-install.\n",
+			      newname ?: mod->modname);
+		} else {
+			close_file(fd);
+			do_command(mod->modname, command, flags & mit_dry_run, error,
+				   "install", cmdline_opts);
+			goto out_optstring;
+		}
 	}
 
 	module = grab_elf_file_fd(mod->filename, fd);
-- 
1.6.4.3


[-- Attachment #2.1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]