Modestas Vainius wrote:
> Hello,
>
> On pirmadienis 28 Rugsėjis 2009 23:30:39 Alan Jenkins wrote:
>
>> Hi, I've fixed some problems I found with the patch you suggested. Is
>> it ok to include your Signed-off-by on this version, to credit you for
>> the idea and the original patch?
>>
>
> 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?
> 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.
The testsuite won't pass with these patches. I'm sitting on a larger
series which updates the testsuite as well. If you're interested in
that, you can find it on github.
<http://github.com/sourcejedi/module-init-tools/commits/master> (web
interface)
<git://github.com/sourcejedi/module-init-tools.git><git://github.com/sourcejedi/module-init-tools.git>
(git clone URL)
Thanks again
Alan
modprobe: Ignore custom install commands if module_in_kernel() doesn't work
Custom install commands rely on a working module_in_kernel() to avoid
an infinite fork-loop. This fails when /sys/module/<module>/initstate
is not available (e.g. before sysfs is mounted). For example:
install snd-pcm /sbin/modprobe --ignore-install snd-pcm && \
{ /sbin/modprobe --quiet snd-pcm-oss ; : ; }
The snd-pcm-oss module depends on snd-pcm. If we can't tell that
snd-pcm is already loaded when we load snd-pcm-oss, we end up
running the entire install command again, ad infinitim.
The original patch needed some fixes
- is_loaded would never be set to -1 due to operator precedence
- passing a warning message to error() would abort the process
when the error function is fatal (use warn() instead).
- the fd of the module file would be leaked.
I have also tried to make the warning message more explicit about what
causes the warning.
Signed-off-by: Modestas Vainius <modestas@xxxxxxxxxx>
Signed-off-by: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx>
diff --git a/modprobe.c b/modprobe.c
index 21a3111..34eaf20 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 already_loaded;
/* Take us off the list. */
list_del(&mod->list);
@@ -1121,8 +1122,9 @@ 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) {
+ already_loaded = module_in_kernel(newname ?: mod->modname, NULL);
+
+ if (!(flags & mit_ignore_loaded) && already_loaded == 1) {
if (flags & mit_first_time)
error("Module %s already in kernel.\n",
newname ?: mod->modname);
@@ -1131,10 +1133,17 @@ 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 (already_loaded == -1) {
+ warn("Failed to read /sys/module/*/initstate.\n");
+ warn("Ignoring install commands for %s"
+ " in case it is already loaded.\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);
modprobe: Get module initstate from /proc/modules if not supported via sysfs
Apparently /sys/module/<module>/initstate only appeared in 2.6.20,
which is still in use.
Copy the original module_in_kernel() verbatim from the GIT archives,
and use it as a fallback if module_in_sysfs() doesn't work.
The original patch suggestion was shorter, but less obvious, adding
more untested code, and turned out to be harder to test.
References: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=524940
Reported-by: Matthew J. Lockner <mlockner@xxxxxxxxxxx> (user)
Reported-by: Modestas Vainius <modestas@xxxxxxxxxx> (original patch)
Signed-off-by: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx>
diff --git a/modprobe.c b/modprobe.c
index 1e1f750..5af7aef 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -519,6 +519,59 @@ static char *add_extra_options(const char *modname,
return optstring;
}
+/* Is module in /proc/modules? If so, fill in usecount if not NULL.
+ 0 means no, 1 means yes, -1 means unknown.
+ */
+static int module_in_procfs(const char *modname, unsigned int *usecount)
+{
+ FILE *proc_modules;
+ char *line;
+
+again:
+ /* 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, usecount is the third entry. */
+ if (!strtok(NULL, " \n"))
+ goto out;
+
+ if (!(entry = strtok(NULL, " \n"))) /* usecount */
+ goto out;
+ else
+ if (usecount)
+ *usecount = atoi(entry);
+
+ /* Followed by - then status. */
+ if (strtok(NULL, " \n")
+ && (entry = strtok(NULL, " \n")) != NULL) {
+ /* No locking, we might hit cases
+ * where module is in flux. Spin. */
+ if (streq(entry, "Loading")
+ || streq(entry, "Unloading")) {
+ usleep(100000);
+ free(line);
+ fclose(proc_modules);
+ goto again;
+ }
+ }
+
+ out:
+ free(line);
+ fclose(proc_modules);
+ return 1;
+ }
+ free(line);
+ }
+ fclose(proc_modules);
+ return 0;
+}
+
/* Read sysfs attribute into a buffer.
* returns: 1 = ok, 0 = attribute missing,
* -1 = file error (or empty file, but we don't care).
@@ -537,10 +590,10 @@ static int read_attribute(const char *filename, char *buf, size_t buflen)
return (s == NULL) ? -1 : 1;
}
-/* Is module in /sys/module? If so, fill in usecount if not NULL.
+/* Is module in /sys/module? If so, fill in usecount if not NULL.
0 means no, 1 means yes, -1 means unknown.
*/
-static int module_in_kernel(const char *modname, unsigned int *usecount)
+static int module_in_sysfs(const char *modname, unsigned int *usecount)
{
int ret;
char *name;
@@ -586,6 +639,22 @@ static int module_in_kernel(const char *modname, unsigned int *usecount)
return 1;
}
+/* Is module loaded? If so, fill in usecount if not NULL.
+ 0 means no, 1 means yes, -1 means unknown.
+ */
+static int module_in_kernel(const char *modname, unsigned int *usecount)
+{
+ int result;
+
+ result = module_in_sysfs(modname, usecount);
+ if (result != -1)
+ return result;
+
+ /* /sys/module/%s/initstate is only available since 2.6.20,
+ fallback to /proc/modules to get module state on earlier kernels. */
+ return module_in_procfs(modname, usecount);
+}
+
void dump_modversions(const char *filename, errfn_t error)
{
struct elf_file *module;
@@ -1134,7 +1203,8 @@ static int insmod(struct list_head *list,
command = find_command(mod->modname, commands);
if (command && !(flags & mit_ignore_commands)) {
if (already_loaded == -1) {
- warn("Failed to read /sys/module/*/initstate.\n");
+ warn("Failed to read /sys/module/*/initstate"
+ " or /proc/modules.\n");
warn("Ignoring install commands for %s"
" in case it is already loaded.\n",
newname ?: mod->modname);
@@ -1223,7 +1293,8 @@ static void rmmod(struct list_head *list,
command = find_command(mod->modname, commands);
if (command && !(flags & mit_ignore_commands)) {
if (exists == -1) {
- warn("Failed to read /sys/module/*/initstate.\n");
+ warn("Failed to read /sys/module/*/initstate"
+ " or /proc/modules.\n");
warn("Ignoring remove commands for %s"
" in case it is not loaded.\n",
mod->modname);
