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]


Modestas Vainius wrote:
> Hello,
>
> On antradienis 29 Rugsėjis 2009 12:12:41 Alan Jenkins wrote:
>   
>>> Is this a replacement for
>>> 0001-Ignore-custom-install-commands-if-failed-to- find-whe.patch [1]?
>>> Then I do not agree with warn("Failed to read
>>> /sys/module/*/initstate.\n"); message. module_in_kernel() might fail due
>>> to other reasons.
>>>       
>> Well, it might fail because /sys/module doesn't exist.  But in that
>> case, it is also true that /sys/module/*/initstate does not exist.  Is
>> that a problem?
>>     
>
> Probably not, but I think the message could be more generic like "Incomplete 
> data in /sys/module/*/ or failed to read /proc/modules".
>
>   
>>> What is more, judging by the warning content above, you do not intend to
>>> apply 0002-Get-module-initstate-from-proc-modules-if-it-is-not-.patch
>>> [1], do you? Well, I don't have strong feelings about whether or not
>>> /proc should be scanned on older (<= 2.6.19) kernels, but another problem
>>> is that current module_in_kernel() will NOT return -1 if initstate is not
>>> present (it will return 0). That's what 0002 patch fixes among added
>>> /proc scanning. So even with the patch you attached, fork-bombing bug
>>> will not be solved. 0002 patch was supposed to be the main patch which
>>> fixes the bug, 0001 was just an additional tweak...
>>>
>>> I will agree with my Signed-off-by on your patch if you let me see (and
>>> test) the patch first. My only interest here is to get the fork bombing
>>> bug solved...
>>>
>>> 1. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=524940#89
>>>       
>> I do plan to submit a patch for /proc/modules support.  Sorry for the
>> confusion; here are the two patches as attachments.  I wrote the second
>> one from scratch, so I'm only asking for your Signed-off-by on the first
>> one.
>>     
>
> 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:

diff --git a/modprobe.c b/modprobe.c
index a0e6600..99ebf78 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -573,8 +573,7 @@ again:
 }
 
 /* Read sysfs attribute into a buffer.
- * returns: 1 = ok, 0 = attribute missing,
- * -1 = file error (or empty file, but we don't care).
+ * returns: 1 = ok, -1 = error (or empty file, but we don't care).
  */
 static int read_attribute(const char *filename, char *buf, size_t buflen)
 {
@@ -583,7 +582,7 @@ static int read_attribute(const char *filename, char *buf, size_t buflen)
 
 	file = fopen(filename, "r");
 	if (file == NULL)
-		return (errno == ENOENT) ? 0 : -1;
+		return -1;
 	s = fgets(buf, buflen, file);
 	fclose(file);
 



--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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