Now we always use stop_machine for module insertion or deletion, we no longer need the modlist_lock: merely disabling preemption is sufficient to block against list manipulation. This avoids deadlock on OOPSen where we can potentially grab the lock twice.
Showing the changes to kernel/module.c (1 of 1 file). Show all
Unified · Split
| @@ -61,10 +61,8 @@ extern int module_sysfs_initialized; · 60 unmodified lines | ||
| 61 | 61 | /* If this is set, the section belongs in the init part of the module */ |
| 62 | 62 | #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1)) |
| 63 | 63 | |
| 64 | −/* Protects module list */ | |
| 65 | −static DEFINE_SPINLOCK(modlist_lock); | |
| 66 | − | |
| 67 | −/* List of modules, protected by module_mutex AND modlist_lock */ | |
| 64 | +/* List of modules, protected by module_mutex or preempt_disable | |
| 65 | + * (add/delete uses stop_machine). */ | |
| 68 | 66 | static DEFINE_MUTEX(module_mutex); |
| 69 | 67 | static LIST_HEAD(modules); |
| 70 | 68 | |
| @@ -760,14 +758,13 @@ static void print_unload_info(struct seq_file *m, struct module *mod) · 689 unmodified lines | ||
| 760 | 758 | void __symbol_put(const char *symbol) |
| 761 | 759 | { |
| 762 | 760 | struct module *owner; |
| 763 | − unsigned long flags; | |
| 764 | 761 | const unsigned long *crc; |
| 765 | 762 | |
| 766 | − spin_lock_irqsave(&modlist_lock, flags); | |
| 763 | + preempt_disable(); | |
| 767 | 764 | if (!__find_symbol(symbol, &owner, &crc, 1)) |
| 768 | 765 | BUG(); |
| 769 | 766 | module_put(owner); |
| 770 | − spin_unlock_irqrestore(&modlist_lock, flags); | |
| 767 | + preempt_enable(); | |
| 771 | 768 | } |
| 772 | 769 | EXPORT_SYMBOL(__symbol_put); |
| 773 | 770 | |
| @@ -1228,14 +1225,14 @@ static void free_module(struct module *mod) · 454 unmodified lines | ||
| 1228 | 1225 | void *__symbol_get(const char *symbol) |
| 1229 | 1226 | { |
| 1230 | 1227 | struct module *owner; |
| 1231 | − unsigned long value, flags; | |
| 1228 | + unsigned long value; | |
| 1232 | 1229 | const unsigned long *crc; |
| 1233 | 1230 | |
| 1234 | − spin_lock_irqsave(&modlist_lock, flags); | |
| 1231 | + preempt_disable(); | |
| 1235 | 1232 | value = __find_symbol(symbol, &owner, &crc, 1); |
| 1236 | 1233 | if (value && !strong_try_module_get(owner)) |
| 1237 | 1234 | value = 0; |
| 1238 | − spin_unlock_irqrestore(&modlist_lock, flags); | |
| 1235 | + preempt_enable(); | |
| 1239 | 1236 | |
| 1240 | 1237 | return (void *)value; |
| 1241 | 1238 | } |
| @@ -2308,11 +2305,10 @@ const struct seq_operations modules_op = { · 1066 unmodified lines | ||
| 2308 | 2305 | /* Given an address, look for it in the module exception tables. */ |
| 2309 | 2306 | const struct exception_table_entry *search_module_extables(unsigned long addr) |
| 2310 | 2307 | { |
| 2311 | − unsigned long flags; | |
| 2312 | 2308 | const struct exception_table_entry *e = NULL; |
| 2313 | 2309 | struct module *mod; |
| 2314 | 2310 | |
| 2315 | − spin_lock_irqsave(&modlist_lock, flags); | |
| 2311 | + preempt_disable(); | |
| 2316 | 2312 | list_for_each_entry(mod, &modules, list) { |
| 2317 | 2313 | if (mod->num_exentries == 0) |
| 2318 | 2314 | continue; |
| @@ -2323,7 +2319,7 @@ const struct exception_table_entry *search_module_extables(unsigned long addr) · 4 unmodified lines | ||
| 2323 | 2319 | if (e) |
| 2324 | 2320 | break; |
| 2325 | 2321 | } |
| 2326 | − spin_unlock_irqrestore(&modlist_lock, flags); | |
| 2322 | + preempt_enable(); | |
| 2327 | 2323 | |
| 2328 | 2324 | /* Now, if we found one, we are running inside it now, hence |
| 2329 | 2325 | we cannot unload the module, hence no refcnt needed. */ |
| @@ -2335,25 +2331,24 @@ const struct exception_table_entry *search_module_extables(unsigned long addr) · 5 unmodified lines | ||
| 2335 | 2331 | */ |
| 2336 | 2332 | int is_module_address(unsigned long addr) |
| 2337 | 2333 | { |
| 2338 | − unsigned long flags; | |
| 2339 | 2334 | struct module *mod; |
| 2340 | 2335 | |
| 2341 | − spin_lock_irqsave(&modlist_lock, flags); | |
| 2336 | + preempt_disable(); | |
| 2342 | 2337 | |
| 2343 | 2338 | list_for_each_entry(mod, &modules, list) { |
| 2344 | 2339 | if (within(addr, mod->module_core, mod->core_size)) { |
| 2345 | − spin_unlock_irqrestore(&modlist_lock, flags); | |
| 2340 | + preempt_enable(); | |
| 2346 | 2341 | return 1; |
| 2347 | 2342 | } |
| 2348 | 2343 | } |
| 2349 | 2344 | |
| 2350 | − spin_unlock_irqrestore(&modlist_lock, flags); | |
| 2345 | + preempt_enable(); | |
| 2351 | 2346 | |
| 2352 | 2347 | return 0; |
| 2353 | 2348 | } |
| 2354 | 2349 | |
| 2355 | 2350 | |
| 2356 | −/* Is this a valid kernel address? We don't grab the lock: we are oopsing. */ | |
| 2351 | +/* Is this a valid kernel address? */ | |
| 2357 | 2352 | struct module *__module_text_address(unsigned long addr) |
| 2358 | 2353 | { |
| 2359 | 2354 | struct module *mod; |
| @@ -2368,11 +2363,10 @@ struct module *__module_text_address(unsigned long addr) · 8 unmodified lines | ||
| 2368 | 2363 | struct module *module_text_address(unsigned long addr) |
| 2369 | 2364 | { |
| 2370 | 2365 | struct module *mod; |
| 2371 | − unsigned long flags; | |
| 2372 | 2366 | |
| 2373 | − spin_lock_irqsave(&modlist_lock, flags); | |
| 2367 | + preempt_disable(); | |
| 2374 | 2368 | mod = __module_text_address(addr); |
| 2375 | − spin_unlock_irqrestore(&modlist_lock, flags); | |
| 2369 | + preempt_enable(); | |
| 2376 | 2370 | |
| 2377 | 2371 | return mod; |
| 2378 | 2372 | } |