Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
Please login to access the resource
openSUSE:Leap:42.3:Staging:A
kmod
0001-Fix-race-while-loading-modules.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0001-Fix-race-while-loading-modules.patch of Package kmod
From 7f17bfb91ff988f2fa6ee794db5170df539e7d95 Mon Sep 17 00:00:00 2001 From: Harish Jenny K N <harish_kandiga@mentor.com> Date: Sun, 22 Feb 2015 15:41:07 -0300 Subject: [PATCH] Fix race while loading modules backport of upstream commit fd44a98ae2eb by mwilck References: bsc#998906 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. ] Signed-off-by: Martin Wilck <mwilck@suse.de> --- libkmod/libkmod-internal.h | 2 ++ libkmod/libkmod-module.c | 40 +++++++++++++++++++++++++++++----------- libkmod/libkmod.c | 33 ++++++++++++++++++++++++++------- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h index 93a00c1..75b1470 100644 --- a/libkmod/libkmod-internal.h +++ b/libkmod/libkmod-internal.h @@ -91,6 +91,7 @@ int kmod_lookup_alias_from_symbols_file(struct kmod_ctx *ctx, const char *name, int kmod_lookup_alias_from_aliases_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3))); int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3))); int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3))); +bool kmod_lookup_alias_is_builtin(struct kmod_ctx *ctx, const char *name) __attribute__((nonnull(1, 2))); int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3))); void kmod_set_modules_visited(struct kmod_ctx *ctx, bool visited) __attribute__((nonnull((1)))); void kmod_set_modules_required(struct kmod_ctx *ctx, bool required) __attribute__((nonnull((1)))); @@ -144,6 +145,7 @@ void kmod_module_set_remove_commands(struct kmod_module *mod, const char *cmd) _ void kmod_module_set_visited(struct kmod_module *mod, bool visited) __attribute__((nonnull(1))); void kmod_module_set_builtin(struct kmod_module *mod, bool builtin) __attribute__((nonnull((1)))); void kmod_module_set_required(struct kmod_module *mod, bool required) __attribute__((nonnull(1))); +bool kmod_module_is_builtin(struct kmod_module *mod) __attribute__((nonnull(1))); /* libkmod-hash.c */ diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index 1d90f34..df926be 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 @@ struct kmod_module { } 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 @@ struct kmod_module { * 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_visited(struct kmod_module *mod, bool visited) 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 @@ void kmod_module_set_required(struct kmod_module *mod, bool required) 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: * @@ -922,7 +938,8 @@ KMOD_EXPORT int kmod_module_apply_filter(const struct kmod_ctx *ctx, 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); @@ -1711,7 +1728,8 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) 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), @@ -1727,7 +1745,7 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) 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", diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c index fe68134..13d0bff 100644 --- a/libkmod/libkmod.c +++ b/libkmod/libkmod.c @@ -480,13 +480,9 @@ int kmod_lookup_alias_from_aliases_file(struct kmod_ctx *ctx, const char *name, name, list); } -int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, - struct kmod_list **list) +static char *lookup_builtin_file(struct kmod_ctx *ctx, const char *name) { - char *line = NULL; - int err = 0; - - assert(*list == NULL); + char *line; if (ctx->indexes[KMOD_INDEX_MODULES_BUILTIN]) { DBG(ctx, "use mmaped index '%s' modname=%s\n", @@ -505,13 +501,25 @@ int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, idx = index_file_open(fn); if (idx == NULL) { DBG(ctx, "could not open builtin file '%s'\n", fn); - goto finish; + return NULL; } line = index_search(idx, name); index_file_close(idx); } + return line; +} + +int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, + struct kmod_list **list) +{ + char *line; + int err = 0; + + assert(*list == NULL); + + line = lookup_builtin_file(ctx, name); if (line != NULL) { struct kmod_module *mod; @@ -522,6 +530,8 @@ int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, goto finish; } + /* already mark it as builtin since it's being created from + * this index */ kmod_module_set_builtin(mod, true); *list = kmod_list_append(*list, mod); if (*list == NULL) @@ -533,6 +543,15 @@ finish: return err; } +bool kmod_lookup_alias_is_builtin(struct kmod_ctx *ctx, const char *name) +{ + _cleanup_free_ char *line; + + line = lookup_builtin_file(ctx, name); + + return line != NULL; +} + char *kmod_search_moddep(struct kmod_ctx *ctx, const char *name) { struct index_file *idx; -- 2.10.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor