Re: [PATCH] modprobe: Ignore custom install commands if module_in_kernel() doesn't work

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Alan Jenkins wrote:
> Alan Jenkins wrote:
>   
>> Modestas Vainius wrote:
>>   
>>     
>>> Hello,
>>>
>>> On antradienis 29 Rugsėjis 2009 12:12:41 Alan Jenkins wrote:
>>>       
>>>       
>>> Probably not, but I think the message could be more generic like "Incomplete 
>>> data in /sys/module/*/ or failed to read /proc/modules".
>>>       

How about this:

WARNING: /sys/module/ not present or too old, and /proc/modules does not
exist.

It doesn't scan as well as yours, but it does

 - imply that you may need to mount /sys/ or /proc/
 - explain why /sys/module/ might not be good enough for modprobe.   If
you don't know about the initstate file, there's no obvious problem with
the old /sys/module/.
 - fit in 80 characters 8-).

>>> Ok, your intentions are much clearer now. But beware, my comment still 
>>> applies. You did not patch code in module_in_sysfs() so it will still return 0 
>>> if /sys/module/<modulename> is present but /sys/module/<modulename>/initstate 
>>> is not present (<= 2.6.19). This is because read_attribute() returns 0 if file 
>>> is NOT present which is the case here. Therefore, module_in_kernel() will not 
>>> fallback to module_in_procfs(), but return 0 and the bug will not be fixed.
>>>   
>>>     
>>>       
>> Ah.  How about if I add this to the patch:
>>     

> Nevermind, that's not quite right.
>   

Ok, I've written a separate patch for this.  It passes the existing
tests, and it works for me on a mock-up of the old sysfs.

Thanks again for your insight
Alan
>From e1000df5ef7ce49d008a915a24f7a01bf928c1b4 Mon Sep 17 00:00:00 2001
From: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx>
Date: Tue, 29 Sep 2009 13:20:36 +0100
Subject: [PATCH] modprobe: Don't assume module absent if no /sys/module/<module>/initstate

/sys/module/<module>/initstate only appeared in 2.6.20.  If it is not
present, we cannot tell whether the module has finished loading
succesfully.  In this case module_in_kernel() should return -1
(undefined).

Take care to to distinguish between "initstate disappeared because the
module was just unloaded" and "initstate is absent but the module is
still present".

"Fork bombing" side effect of this bug is described in [1].
A followup patch will add a fallback to /proc/modules.

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

Signed-off-by: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx>
Reported-by: Modestas Vainius <modestas@xxxxxxxxxx>
---
 modprobe.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/modprobe.c b/modprobe.c
index e624367..d60dc43 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -560,14 +560,26 @@ static int module_in_kernel(const char *modname, unsigned int *usecount)
 	if (ret < 0)
 		return (errno == ENOENT) ? 0 : -1; /* Not found or unknown. */
 
-	/* Wait for the existing module to either go live or disappear. */
 	nofail_asprintf(&name, "/sys/module/%s/initstate", modname);
-	while (1) {
-		ret = read_attribute(name, attr, ATTR_LEN);
-		if (ret != 1 || streq(attr, "live\n"))
-			break;
+	ret = read_attribute(name, attr, ATTR_LEN);
+	if (ret == 0) {
+		free(name);
+		nofail_asprintf(&name, "/sys/module/%s", modname);
+		if (stat(name, &finfo) < 0) {
+			/* module was removed before we could read initstate */
+			ret = 0;
+		} else {
+			/* initstate not available (2.6.19 or earlier) */
+			ret = -1;
+		}
+		free(name);
+		return ret;
+	}
 
+	/* Wait for the existing module to either go live or disappear. */
+	while (ret == 1 && !streq(attr, "live\n")) {
 		usleep(100000);
+		ret = read_attribute(name, attr, ATTR_LEN);
 	}
 	free(name);
 
-- 
1.6.3.2


[Home]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Video Projectors]     [PDAs]     [Free Online Dating]     [Hacking TiVo]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Devices]     [Big List of Linux Books]     [16.7MP]

Add to Google Powered by Linux