Fix race while loading modules
usecase: two sd cards are being mounted in parallel at same time on
dual core. example modules which are getting loaded is nls_cp437.
While one module is being loaded , it starts creating sysfs files.
meanwhile on other core, modprobe might return saying the module
is KMOD_MODULE_BUILTIN, which might result in not mounting sd card.
Experiments done to prove the issue in kmod.
Added sleep in kernel module.c at the place of creation of sysfs files.
Then tried `modprobe nls_cp437` from two different shells.
While the first was still waiting for its completion ,
the second one returned saying the module is built-in.
[ Lucas:
The problem is that the creation of /sys/module/<name> and
/sys/module/<name>/initstate are not atomic. There's a small window in
which the directory exists but the initstate file was still not
created.
Built-in modules can be handled by searching the modules.builtin file.
We actually lose some "modules" that create entries in /sys/modules
(e.g. vt) and are not in modules.builtin file: only those that can be
compiled as module are present in this file.
We enforce mod->builtin to always be up-to-date when
kmod_module_get_initstate() is called. This way if the directory
exists but the initstate doesn't, we can be sure this is because the
module is in the "coming" state, i.e. kernel didn't create the file
yet, but since builtin modules were already handled by checking our
index the only reason for that to happen is that we hit the race
condition.
I also added some tweaks to the patch, so we don't repeat the code for builtin
lookup. ]
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 30f15ca..366308f 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -49,6 +49,12 @@
* @short_description: operate on kernel modules
*/
+enum kmod_module_builtin {
+ KMOD_MODULE_BUILTIN_UNKNOWN,
+ KMOD_MODULE_BUILTIN_NO,
+ KMOD_MODULE_BUILTIN_YES,
+};
+
/**
* kmod_module:
*
@@ -75,6 +81,13 @@
} init;
/*
+ * mark if module is builtin, i.e. it's present on modules.builtin
+ * file. This is set as soon as it is needed or as soon as we know
+ * about it, i.e. the module was created from builtin lookup.
+ */
+ enum kmod_module_builtin builtin;
+
+ /*
* private field used by kmod_module_get_probe_list() to detect
* dependency loops
*/
@@ -92,13 +105,6 @@
* is a softdep only
*/
bool required : 1;
-
- /*
- * if module was created by searching the modules.builtin file, this
- * is set. There's nothing much useful one can do with such a
- * "module", except knowing it's builtin.
- */
- bool builtin : 1;
};
static inline const char *path_join(const char *path, size_t prefixlen,
@@ -212,7 +218,8 @@
void kmod_module_set_builtin(struct kmod_module *mod, bool builtin)
{
- mod->builtin = builtin;
+ mod->builtin =
+ builtin ? KMOD_MODULE_BUILTIN_YES : KMOD_MODULE_BUILTIN_NO;
}
void kmod_module_set_required(struct kmod_module *mod, bool required)
@@ -220,6 +227,15 @@
mod->required = required;
}
+bool kmod_module_is_builtin(struct kmod_module *mod)
+{
+ if (mod->builtin == KMOD_MODULE_BUILTIN_UNKNOWN) {
+ kmod_module_set_builtin(mod,
+ kmod_lookup_alias_is_builtin(mod->ctx, mod->name));
+ }
+
+ return mod->builtin == KMOD_MODULE_BUILTIN_YES;
+}
/*
* Memory layout with alias:
*
@@ -924,7 +940,8 @@
module_is_blacklisted(mod))
continue;
- if ((filter_type & KMOD_FILTER_BUILTIN) && mod->builtin)
+ if ((filter_type & KMOD_FILTER_BUILTIN)
+ && kmod_module_is_builtin(mod))
continue;
node = kmod_list_append(*output, mod);
@@ -1713,7 +1730,8 @@
if (mod == NULL)
return -ENOENT;
- if (mod->builtin)
+ /* remove const: this can only change internal state */
+ if (kmod_module_is_builtin((struct kmod_module *)mod))
return KMOD_MODULE_BUILTIN;
pathlen = snprintf(path, sizeof(path),
@@ -1729,7 +1747,7 @@
struct stat st;
path[pathlen - (sizeof("/initstate") - 1)] = '\0';
if (stat(path, &st) == 0 && S_ISDIR(st.st_mode))
- return KMOD_MODULE_BUILTIN;
+ return KMOD_MODULE_COMING;
}
DBG(mod->ctx, "could not open '%s': %s\n",