+++ Miroslav Benes [27/10/20 15:03 +0100]:
If a module fails to load due to an error in prepare_coming_module(), the following error handling in load_module() runs with MODULE_STATE_COMING in module's state. Fix it by correctly setting MODULE_STATE_GOING under "bug_cleanup" label.Signed-off-by: Miroslav Benes <mbe...@suse.cz> --- kernel/module.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/module.c b/kernel/module.c index a4fa44a652a7..b34235082394 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3991,6 +3991,7 @@ static int load_module(struct load_info *info, const char __user *uargs, MODULE_STATE_GOING, mod); klp_module_going(mod); bug_cleanup: + mod->state = MODULE_STATE_GOING; /* module_bug_cleanup needs module_mutex protection */ mutex_lock(&module_mutex); module_bug_cleanup(mod);
Thanks for the fix! Hmm, I am wondering if we also need to set the module to GOING if it happens to fail while it is still UNFORMED. Currently, when a module is UNFORMED and encounters an error during load_module(), it stays UNFORMED until it finally goes away. That sounds fine, but try_module_get() technically permits you to get a module while it's UNFORMED (but not if it's GOING). Theoretically someone could increase the refcount of an unformed module that has encountered an error condition and is in the process of going away. This shouldn't happen if we properly set the module to GOING whenever it encounters an error during load_module(). But - I cannot think of a scenario where someone could call try_module_get() on an unformed module, since find_module() etc. do not return unformed modules, so they shouldn't be visible outside of the module loader. So in practice, I think we're probably safe here..
