| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 31 May 2010 13:17:17 -0700 (PDT)
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Andrew Morton <akpm@...ux-foundation.org>
cc: Rusty Russell <rusty@...tcorp.com.au>,
Brandon Philips <brandon@...p.org>,
"Rafael J. Wysocki" <rjw@...k.pl>,
LKML <linux-kernel@...r.kernel.org>,
Jon Masters <jonathan@...masters.org>,
Tejun Heo <htejun@...il.com>,
Masami Hiramatsu <mhiramat@...hat.com>,
Kay Sievers <kay.sievers@...y.org>
Subject: [PATCH 2/2] module: wait for other modules after dropping the
module_mutex
From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Mon, 31 May 2010 13:09:21 -0700
Subject: [PATCH 2/2] module: wait for other modules after dropping the module_mutex
Instead of doing a "use_module()" when linking to other modules, we do a
simpler link_module() that does not require the dependent modules to be
initialized. We then wait for all dependent modules _after_ we have
dropped the module_mutex lock, which avoids a deadlock.
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
---
This is the patch that should hopefully fix Brandon's deadlock. But I
haven't really tested that case - so who knows?
kernel/module.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 82ae1e4..c95f97e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -611,6 +611,54 @@ int use_module(struct module *a, struct module *b)
}
EXPORT_SYMBOL_GPL(use_module);
+/* Module a links to b */
+static int link_module(struct module *a, struct module *b)
+{
+ if (b == NULL || already_uses(a, b))
+ return 1;
+ if (!try_module_get(b))
+ return 0;
+ return add_module_usage(a, b);
+}
+
+/*
+ * Are all modules LIVE? Return 0 (ok) if so.
+ * Return -EINVAL if any are going, and EAGAIN
+ * if there are modules still initializing.
+ */
+static int all_modules_ok(struct module *mod)
+{
+ int err = 0;
+ struct module_use *use;
+
+ mutex_lock(&module_mutex);
+ list_for_each_entry(use, &mod->target_list, target_list) {
+ struct module *target = use->target;
+ switch (target->state) {
+ case MODULE_STATE_LIVE:
+ break;
+ case MODULE_STATE_COMING:
+ if (!err)
+ err = -EAGAIN;
+ break;
+ default:
+ err = -EINVAL;
+ }
+ }
+ mutex_unlock(&module_mutex);
+ return err;
+}
+
+/* Wait for dependent modules to finish */
+static int module_load_wait(struct module *mod)
+{
+ int err;
+
+ if (wait_event_interruptible_timeout(module_wq, (err = all_modules_ok(mod)) != -EAGAIN, 30 * HZ) <= 0)
+ printk("%s: gave up waiting for init of modules.\n", mod->name);
+ return err;
+}
+
/* Clear the unload stuff of the module. */
static void module_unload_free(struct module *mod)
{
@@ -898,6 +946,21 @@ int use_module(struct module *a, struct module *b)
}
EXPORT_SYMBOL_GPL(use_module);
+/* FIXME! This is wrong. We should do the full link_module. Otherwise
+ * we'll fail if a module we depend on just happens to be loading */
+static int link_module(struct module *a, struct module *b)
+{
+ return strong_try_module_get(b) == 0;
+}
+
+/* Wait for dependent modules to finish. See the FIXME on link_module()
+ * above: we've required that the modules were initialized too early,
+ * so there is nothing to wait on now (nor do we maintain any lists) */
+static int module_load_wait(struct module *mod)
+{
+ return 0;
+}
+
static inline void module_unload_init(struct module *mod)
{
}
@@ -1068,11 +1131,11 @@ static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs,
sym = find_symbol(name, &owner, &crc,
!(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true);
- /* use_module can fail due to OOM,
+ /* link_module can fail due to OOM,
or module initialization or unloading */
if (sym) {
if (!check_version(sechdrs, versindex, name, mod, crc, owner)
- || !use_module(mod, owner))
+ || !link_module(mod, owner))
sym = NULL;
}
return sym;
@@ -2528,6 +2591,14 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
/* Drop lock so they can recurse */
mutex_unlock(&module_mutex);
+ /*
+ * Now that we've released the module lock, wait for dependent
+ * modules to finish loading
+ */
+ ret = module_load_wait(mod);
+ if (ret)
+ goto unload;
+
blocking_notifier_call_chain(&module_notify_list,
MODULE_STATE_COMING, mod);
@@ -2535,20 +2606,8 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
/* Start the module */
if (mod->init != NULL)
ret = do_one_initcall(mod->init);
- if (ret < 0) {
- /* Init routine failed: abort. Try to protect us from
- buggy refcounters. */
- mod->state = MODULE_STATE_GOING;
- synchronize_sched();
- module_put(mod);
- blocking_notifier_call_chain(&module_notify_list,
- MODULE_STATE_GOING, mod);
- mutex_lock(&module_mutex);
- free_module(mod);
- mutex_unlock(&module_mutex);
- wake_up(&module_wq);
- return ret;
- }
+ if (ret < 0)
+ goto unload;
if (ret > 0) {
printk(KERN_WARNING
"%s: '%s'->init suspiciously returned %d, it should follow 0/-E convention\n"
@@ -2583,6 +2642,20 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
mutex_unlock(&module_mutex);
return 0;
+
+unload:
+ /* Initialization failed: abort. Try to protect us from
+ buggy refcounters. */
+ mod->state = MODULE_STATE_GOING;
+ synchronize_sched();
+ module_put(mod);
+ blocking_notifier_call_chain(&module_notify_list,
+ MODULE_STATE_GOING, mod);
+ mutex_lock(&module_mutex);
+ free_module(mod);
+ mutex_unlock(&module_mutex);
+ wake_up(&module_wq);
+ return ret;
}
static inline int within(unsigned long addr, void *start, unsigned long size)
--
1.7.1.227.gb676f
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists