module: fix symbol waiting when module fails before init

We use resolve_symbol_wait(), which blocks if the module containing the symbol is still loading. However: 1) The module_wq we use is only woken after calling the modules' init function, but there are other failure paths after the module is placed in the linked list where we need to do the same thing. 2) wake_up() only wakes one waiter, and our waitqueue is shared by all modules, so we need to wake them all. 3) wake_up_all() doesn't imply a memory barrier: I feel happier calling it after we've grabbed and dropped the module_mutex, not just after the state assignment. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

module: fix symbol waiting when module fails before init
We use resolve_symbol_wait(), which blocks if the module containing the symbol is still loading. However: 1) The module_wq we use is only woken after calling the modules' init function, but there are other failure paths after the module is placed in the linked list where we need to do the same thing. 2) wake_up() only wakes one waiter, and our waitqueue is shared by all modules, so we need to wake them all. 3) wake_up_all() doesn't imply a memory barrier: I feel happier calling it after we've grabbed and dropped the module_mutex, not just after the state assignment. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
1 parent 786d35d45c
Showing 1 changed file with 4 additions and 4 deletions Side-by-side Diff
kernel/module.c
@@ -2959,7 +2959,7 @@
 	/* Unlink carefully: kallsyms could be walking list. */
 	list_del_rcu(&mod->list);
 	module_bug_cleanup(mod);
-
+	wake_up_all(&module_wq);
  ddebug:
 	dynamic_debug_remove(info.debug);
  unlock:
@@ -3034,7 +3034,7 @@
 		blocking_notifier_call_chain(&module_notify_list,
 					     MODULE_STATE_GOING, mod);
 		free_module(mod);
-		wake_up(&module_wq);
+		wake_up_all(&module_wq);
 		return ret;
 	}
 	if (ret > 0) {
  
@@ -3046,9 +3046,8 @@
 		dump_stack();
 	}
  
-	/* Now it's a first class citizen!  Wake up anyone waiting for it. */
+	/* Now it's a first class citizen! */
 	mod->state = MODULE_STATE_LIVE;
-	wake_up(&module_wq);
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_LIVE, mod);
  
@@ -3071,6 +3070,7 @@
 	mod->init_ro_size = 0;
 	mod->init_text_size = 0;
 	mutex_unlock(&module_mutex);
+	wake_up_all(&module_wq);
  
 	return 0;
 }
...	...	@@ -2959,7 +2959,7 @@
2959	2959	/* Unlink carefully: kallsyms could be walking list. */
2960	2960	list_del_rcu(&mod->list);
2961	2961	module_bug_cleanup(mod);
2962		-
	2962	+ wake_up_all(&module_wq);
2963	2963	ddebug:
2964	2964	dynamic_debug_remove(info.debug);
2965	2965	unlock:
...	...	@@ -3034,7 +3034,7 @@
3034	3034	blocking_notifier_call_chain(&module_notify_list,
3035	3035	MODULE_STATE_GOING, mod);
3036	3036	free_module(mod);
3037		- wake_up(&module_wq);
	3037	+ wake_up_all(&module_wq);
3038	3038	return ret;
3039	3039	}
3040	3040	if (ret > 0) {
3041	3041
...	...	@@ -3046,9 +3046,8 @@
3046	3046	dump_stack();
3047	3047	}
3048	3048
3049		- /* Now it's a first class citizen! Wake up anyone waiting for it. */
	3049	+ /* Now it's a first class citizen! */
3050	3050	mod->state = MODULE_STATE_LIVE;
3051		- wake_up(&module_wq);
3052	3051	blocking_notifier_call_chain(&module_notify_list,
3053	3052	MODULE_STATE_LIVE, mod);
3054	3053
...	...	@@ -3071,6 +3070,7 @@
3071	3070	mod->init_ro_size = 0;
3072	3071	mod->init_text_size = 0;
3073	3072	mutex_unlock(&module_mutex);
	3073	+ wake_up_all(&module_wq);
3074	3074
3075	3075	return 0;
3076	3076	}