[PATCH] module: drain external refs before freeing a failed-init module

[PATCH] module: drain external refs before freeing a failed-init module

[Mahanta Jambigi]

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 <mjambigi@linux.ibm.com>
---
Link: https://lore.kernel.org/all/5dc1fe2b-289e-4786-b9e9-e181dda8d4e9@linux.ibm.com/
      [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