When a module's __init makes it externally reachable (e.g. by registering a
protocol family or a bus driver) and a later __init step fails, do_init_module()
frees the module while callers that entered through that interface and called
try_module_get() may still be executing module text.
The normal unload path in try_stop_module() refuses to proceed while
module_refcount() is non-zero. The init-failure path has no such check and races
to free_module() immediately after synchronize_rcu().
Fix it by waiting for all external references to drain before calling
free_module(). Add a per-module struct completion refs_gone inside the existing
CONFIG_MODULE_UNLOAD block that is initialised in module_unload_init() and
completed from module_put() when refcnt drops back to MODULE_REF_BASE and the
module state is MODULE_STATE_GOING. The fail: path calls wait_for_completion()
(guarded by the same #ifdef) after dropping the loader's own reference,
replacing the previous implicit assumption that no external refs exist at that
point.
Also move wake_up_all(&module_wq) to before the drain so that concurrent loaders
blocked in module_patient_check_exists() are unblocked immediately once
MODULE_STATE_GOING is visible, rather than after the drain completes.
Fixes: 1fb9341ac348 ("module: put modules in list much earlier.")
Signed-off-by: Mahanta Jambigi <[email protected]>
---
Link:
https://lore.kernel.org/all/[email protected]/
[RFC] module: init-failure path can free a module with live
try_module_get() users
include/linux/module.h | 2 ++
kernel/module/main.c | 33 +++++++++++++++++++++++++++-----
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 96cc98568eea..0dee2ed02a2d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -562,6 +562,8 @@ struct module {
void (*exit)(void);
atomic_t refcnt;
+ /* Completed when refcnt drops to MODULE_REF_BASE on init failure. */
+ struct completion refs_gone;
#endif
#ifdef CONFIG_CONSTRUCTORS
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c1b34dc1e89a..655c693bd8e2 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -655,6 +655,7 @@ static int module_unload_init(struct module *mod)
* refcnt == 0 means module is going.
*/
atomic_set(&mod->refcnt, MODULE_REF_BASE);
+ init_completion(&mod->refs_gone);
INIT_LIST_HEAD(&mod->source_list);
INIT_LIST_HEAD(&mod->target_list);
@@ -956,6 +957,14 @@ void module_put(struct module *module)
ret = atomic_dec_if_positive(&module->refcnt);
WARN_ON(ret < 0); /* Failed to put refcount */
trace_module_put(module, _RET_IP_);
+ /*
+ * If refcnt just dropped to MODULE_REF_BASE and the module is
going
+ * away due to an init failure, all external references are
gone. Wake
+ * any thread waiting in the init-failure drain path.
+ */
+ if (ret == MODULE_REF_BASE &&
+ module->state == MODULE_STATE_GOING)
+ complete(&module->refs_gone);
}
}
EXPORT_SYMBOL(module_put);
@@ -3206,16 +3214,36 @@ static noinline int do_init_module(struct module *mod)
fail_free_freeinit:
kfree(freeinit);
fail:
- /* Try to protect us from buggy refcounters. */
mod->state = MODULE_STATE_GOING;
synchronize_rcu();
+
+ /*
+ * Unblock concurrent loaders waiting in module_patient_check_exists()
on
+ * module_wq. finished_loading() returns true once state is GOING, so
they
+ * can now proceed. Do this before the drain below so a failed init
delays
+ * only this task, not every other loader that raced to load the same
+ * module.
+ */
+ wake_up_all(&module_wq);
+
+ /* Drop the loader's own reference taken in module_unload_init(). */
module_put(mod);
+
+#ifdef CONFIG_MODULE_UNLOAD
+ /*
+ * Wait for all external references taken before MODULE_STATE_GOING
became
+ * visible to be released. module_put() completes refs_gone when refcnt
+ * drops to MODULE_REF_BASE. The hung-task detector will catch any
module
+ * whose error path leaks a reference.
+ */
+ wait_for_completion(&mod->refs_gone);
+#endif
+
blocking_notifier_call_chain(&module_notify_list,
MODULE_STATE_GOING, mod);
klp_module_going(mod);
ftrace_release_mod(mod);
free_module(mod);
- wake_up_all(&module_wq);
return ret;
}
--
2.43.7
