modules: de-mutex more symbol lookup paths in the module code

Kyle McMartin reports sysrq_timer_list_show() can hit the module mutex from hard interrupt context. These paths don't need to though, since we long ago changed all the module list manipulation to occur via stop_machine(). Disabling preemption is enough. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Cc: Ingo Molnar <mingo@elte.hu> Cc: Kyle McMartin <kyle@mcmartin.ca> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

modules: de-mutex more symbol lookup paths in the module code
Kyle McMartin reports sysrq_timer_list_show() can hit the module mutex from hard interrupt context. These paths don't need to though, since we long ago changed all the module list manipulation to occur via stop_machine(). Disabling preemption is enough. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Cc: Ingo Molnar <mingo@elte.hu> Cc: Kyle McMartin <kyle@mcmartin.ca> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Rusty Russell · Linus Torvalds
1 parent 1a1b285c24
Showing 1 changed file with 18 additions and 11 deletions Side-by-side Diff
kernel/module.c
@@ -2214,29 +2214,34 @@
 /* For kallsyms to ask for address resolution.  NULL means not found.
    We don't lock, as this is used for oops resolution and races are a
    lesser concern. */
+/* FIXME: Risky: returns a pointer into a module w/o lock */
 const char *module_address_lookup(unsigned long addr,
 				  unsigned long *size,
 				  unsigned long *offset,
 				  char **modname)
 {
 	struct module *mod;
+	const char *ret = NULL;
  
+	preempt_disable();
 	list_for_each_entry(mod, &modules, list) {
 		if (within(addr, mod->module_init, mod->init_size)
 		    || within(addr, mod->module_core, mod->core_size)) {
 			if (modname)
 				*modname = mod->name;
-			return get_ksymbol(mod, addr, size, offset);
+			ret = get_ksymbol(mod, addr, size, offset);
+			break;
 		}
 	}
-	return NULL;
+	preempt_enable();
+	return ret;
 }
  
 int lookup_module_symbol_name(unsigned long addr, char *symname)
 {
 	struct module *mod;
  
-	mutex_lock(&module_mutex);
+	preempt_disable();
 	list_for_each_entry(mod, &modules, list) {
 		if (within(addr, mod->module_init, mod->init_size) ||
 		    within(addr, mod->module_core, mod->core_size)) {
  
@@ -2246,12 +2251,12 @@
 			if (!sym)
 				goto out;
 			strlcpy(symname, sym, KSYM_NAME_LEN);
-			mutex_unlock(&module_mutex);
+			preempt_enable();
 			return 0;
 		}
 	}
 out:
-	mutex_unlock(&module_mutex);
+	preempt_enable();
 	return -ERANGE;
 }
  
@@ -2260,7 +2265,7 @@
 {
 	struct module *mod;
  
-	mutex_lock(&module_mutex);
+	preempt_disable();
 	list_for_each_entry(mod, &modules, list) {
 		if (within(addr, mod->module_init, mod->init_size) ||
 		    within(addr, mod->module_core, mod->core_size)) {
  
@@ -2273,12 +2278,12 @@
 				strlcpy(modname, mod->name, MODULE_NAME_LEN);
 			if (name)
 				strlcpy(name, sym, KSYM_NAME_LEN);
-			mutex_unlock(&module_mutex);
+			preempt_enable();
 			return 0;
 		}
 	}
 out:
-	mutex_unlock(&module_mutex);
+	preempt_enable();
 	return -ERANGE;
 }
  
@@ -2287,7 +2292,7 @@
 {
 	struct module *mod;
  
-	mutex_lock(&module_mutex);
+	preempt_disable();
 	list_for_each_entry(mod, &modules, list) {
 		if (symnum < mod->num_symtab) {
 			*value = mod->symtab[symnum].st_value;
  
@@ -2296,12 +2301,12 @@
 				KSYM_NAME_LEN);
 			strlcpy(module_name, mod->name, MODULE_NAME_LEN);
 			*exported = is_exported(name, mod);
-			mutex_unlock(&module_mutex);
+			preempt_enable();
 			return 0;
 		}
 		symnum -= mod->num_symtab;
 	}
-	mutex_unlock(&module_mutex);
+	preempt_enable();
 	return -ERANGE;
 }
  
@@ -2324,6 +2329,7 @@
 	unsigned long ret = 0;
  
 	/* Don't lock: we're in enough trouble already. */
+	preempt_disable();
 	if ((colon = strchr(name, ':')) != NULL) {
 		*colon = '\0';
 		if ((mod = find_module(name)) != NULL)
@@ -2334,6 +2340,7 @@
 			if ((ret = mod_find_symname(mod, name)) != 0)
 				break;
 	}
+	preempt_enable();
 	return ret;
 }
 #endif /* CONFIG_KALLSYMS */
...	...	@@ -2214,29 +2214,34 @@
2214	2214	/* For kallsyms to ask for address resolution. NULL means not found.
2215	2215	We don't lock, as this is used for oops resolution and races are a
2216	2216	lesser concern. */
	2217	+/* FIXME: Risky: returns a pointer into a module w/o lock */
2217	2218	const char *module_address_lookup(unsigned long addr,
2218	2219	unsigned long *size,
2219	2220	unsigned long *offset,
2220	2221	char **modname)
2221	2222	{
2222	2223	struct module *mod;
	2224	+ const char *ret = NULL;
2223	2225
	2226	+ preempt_disable();
2224	2227	list_for_each_entry(mod, &modules, list) {
2225	2228	if (within(addr, mod->module_init, mod->init_size)
2226	2229	\|\| within(addr, mod->module_core, mod->core_size)) {
2227	2230	if (modname)
2228	2231	*modname = mod->name;
2229		- return get_ksymbol(mod, addr, size, offset);
	2232	+ ret = get_ksymbol(mod, addr, size, offset);
	2233	+ break;
2230	2234	}
2231	2235	}
2232		- return NULL;
	2236	+ preempt_enable();
	2237	+ return ret;
2233	2238	}
2234	2239
2235	2240	int lookup_module_symbol_name(unsigned long addr, char *symname)
2236	2241	{
2237	2242	struct module *mod;
2238	2243
2239		- mutex_lock(&module_mutex);
	2244	+ preempt_disable();
2240	2245	list_for_each_entry(mod, &modules, list) {
2241	2246	if (within(addr, mod->module_init, mod->init_size) \|\|
2242	2247	within(addr, mod->module_core, mod->core_size)) {
2243	2248
...	...	@@ -2246,12 +2251,12 @@
2246	2251	if (!sym)
2247	2252	goto out;
2248	2253	strlcpy(symname, sym, KSYM_NAME_LEN);
2249		- mutex_unlock(&module_mutex);
	2254	+ preempt_enable();
2250	2255	return 0;
2251	2256	}
2252	2257	}
2253	2258	out:
2254		- mutex_unlock(&module_mutex);
	2259	+ preempt_enable();
2255	2260	return -ERANGE;
2256	2261	}
2257	2262
...	...	@@ -2260,7 +2265,7 @@
2260	2265	{
2261	2266	struct module *mod;
2262	2267
2263		- mutex_lock(&module_mutex);
	2268	+ preempt_disable();
2264	2269	list_for_each_entry(mod, &modules, list) {
2265	2270	if (within(addr, mod->module_init, mod->init_size) \|\|
2266	2271	within(addr, mod->module_core, mod->core_size)) {
2267	2272
...	...	@@ -2273,12 +2278,12 @@
2273	2278	strlcpy(modname, mod->name, MODULE_NAME_LEN);
2274	2279	if (name)
2275	2280	strlcpy(name, sym, KSYM_NAME_LEN);
2276		- mutex_unlock(&module_mutex);
	2281	+ preempt_enable();
2277	2282	return 0;
2278	2283	}
2279	2284	}
2280	2285	out:
2281		- mutex_unlock(&module_mutex);
	2286	+ preempt_enable();
2282	2287	return -ERANGE;
2283	2288	}
2284	2289
...	...	@@ -2287,7 +2292,7 @@
2287	2292	{
2288	2293	struct module *mod;
2289	2294
2290		- mutex_lock(&module_mutex);
	2295	+ preempt_disable();
2291	2296	list_for_each_entry(mod, &modules, list) {
2292	2297	if (symnum < mod->num_symtab) {
2293	2298	*value = mod->symtab[symnum].st_value;
2294	2299
...	...	@@ -2296,12 +2301,12 @@
2296	2301	KSYM_NAME_LEN);
2297	2302	strlcpy(module_name, mod->name, MODULE_NAME_LEN);
2298	2303	*exported = is_exported(name, mod);
2299		- mutex_unlock(&module_mutex);
	2304	+ preempt_enable();
2300	2305	return 0;
2301	2306	}
2302	2307	symnum -= mod->num_symtab;
2303	2308	}
2304		- mutex_unlock(&module_mutex);
	2309	+ preempt_enable();
2305	2310	return -ERANGE;
2306	2311	}
2307	2312
...	...	@@ -2324,6 +2329,7 @@
2324	2329	unsigned long ret = 0;
2325	2330
2326	2331	/* Don't lock: we're in enough trouble already. */
	2332	+ preempt_disable();
2327	2333	if ((colon = strchr(name, ':')) != NULL) {
2328	2334	*colon = '\0';
2329	2335	if ((mod = find_module(name)) != NULL)
...	...	@@ -2334,6 +2340,7 @@
2334	2340	if ((ret = mod_find_symname(mod, name)) != 0)
2335	2341	break;
2336	2342	}
	2343	+ preempt_enable();
2337	2344	return ret;
2338	2345	}
2339	2346	#endif /* CONFIG_KALLSYMS */