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
