tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

modules: Fix module_bug_list list corruption race

With all the recent module loading cleanups, we've minimized the code
that sits under module_mutex, fixing various deadlocks and making it
possible to do most of the module loading in parallel.

However, that whole conversion totally missed the rather obscure code
that adds a new module to the list for BUG() handling. That code was
doubly obscure because (a) the code itself lives in lib/bugs.c (for
dubious reasons) and (b) it gets called from the architecture-specific
"module_finalize()" rather than from generic code.

Calling it from arch-specific code makes no sense what-so-ever to begin
with, and is now actively wrong since that code isn't protected by the
module loading lock any more.

So this commit moves the "module_bug_{finalize,cleanup}()" calls away
from the arch-specific code, and into the generic code - and in the
process protects it with the module_mutex so that the list operations
are now safe.

Future fixups:
- move the module list handling code into kernel/module.c where it
belongs.
- get rid of 'module_bug_list' and just use the regular list of modules
(called 'modules' - imagine that) that we already create and maintain
for other reasons.

Reported-and-tested-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Adrian Bunk <bunk@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Linus Torvalds 5336377d 2f6b3aa7

+14 -26
+1 -2
arch/avr32/kernel/module.c
··· 314 314 vfree(module->arch.syminfo); 315 315 module->arch.syminfo = NULL; 316 316 317 - return module_bug_finalize(hdr, sechdrs, module); 317 + return 0; 318 318 } 319 319 320 320 void module_arch_cleanup(struct module *module) 321 321 { 322 - module_bug_cleanup(module); 323 322 }
+1 -2
arch/h8300/kernel/module.c
··· 112 112 const Elf_Shdr *sechdrs, 113 113 struct module *me) 114 114 { 115 - return module_bug_finalize(hdr, sechdrs, me); 115 + return 0; 116 116 } 117 117 118 118 void module_arch_cleanup(struct module *mod) 119 119 { 120 - module_bug_cleanup(mod); 121 120 }
+1 -2
arch/mn10300/kernel/module.c
··· 206 206 const Elf_Shdr *sechdrs, 207 207 struct module *me) 208 208 { 209 - return module_bug_finalize(hdr, sechdrs, me); 209 + return 0; 210 210 } 211 211 212 212 /* ··· 214 214 */ 215 215 void module_arch_cleanup(struct module *mod) 216 216 { 217 - module_bug_cleanup(mod); 218 217 }
+1 -2
arch/parisc/kernel/module.c
··· 941 941 nsyms = newptr - (Elf_Sym *)symhdr->sh_addr; 942 942 DEBUGP("NEW num_symtab %lu\n", nsyms); 943 943 symhdr->sh_size = nsyms * sizeof(Elf_Sym); 944 - return module_bug_finalize(hdr, sechdrs, me); 944 + return 0; 945 945 } 946 946 947 947 void module_arch_cleanup(struct module *mod) 948 948 { 949 949 deregister_unwind_table(mod); 950 - module_bug_cleanup(mod); 951 950 }
-5
arch/powerpc/kernel/module.c
··· 65 65 const Elf_Shdr *sect; 66 66 int err; 67 67 68 - err = module_bug_finalize(hdr, sechdrs, me); 69 - if (err) 70 - return err; 71 - 72 68 /* Apply feature fixups */ 73 69 sect = find_section(hdr, sechdrs, "__ftr_fixup"); 74 70 if (sect != NULL) ··· 97 101 98 102 void module_arch_cleanup(struct module *mod) 99 103 { 100 - module_bug_cleanup(mod); 101 104 }
+1 -2
arch/s390/kernel/module.c
··· 407 407 { 408 408 vfree(me->arch.syminfo); 409 409 me->arch.syminfo = NULL; 410 - return module_bug_finalize(hdr, sechdrs, me); 410 + return 0; 411 411 } 412 412 413 413 void module_arch_cleanup(struct module *mod) 414 414 { 415 - module_bug_cleanup(mod); 416 415 }
-2
arch/sh/kernel/module.c
··· 149 149 int ret = 0; 150 150 151 151 ret |= module_dwarf_finalize(hdr, sechdrs, me); 152 - ret |= module_bug_finalize(hdr, sechdrs, me); 153 152 154 153 return ret; 155 154 } 156 155 157 156 void module_arch_cleanup(struct module *mod) 158 157 { 159 - module_bug_cleanup(mod); 160 158 module_dwarf_cleanup(mod); 161 159 }
+1 -2
arch/x86/kernel/module.c
··· 239 239 apply_paravirt(pseg, pseg + para->sh_size); 240 240 } 241 241 242 - return module_bug_finalize(hdr, sechdrs, me); 242 + return 0; 243 243 } 244 244 245 245 void module_arch_cleanup(struct module *mod) 246 246 { 247 247 alternatives_smp_module_del(mod); 248 - module_bug_cleanup(mod); 249 248 }
+2 -3
include/linux/module.h
··· 686 686 687 687 688 688 #ifdef CONFIG_GENERIC_BUG 689 - int module_bug_finalize(const Elf_Ehdr *, const Elf_Shdr *, 689 + void module_bug_finalize(const Elf_Ehdr *, const Elf_Shdr *, 690 690 struct module *); 691 691 void module_bug_cleanup(struct module *); 692 692 693 693 #else /* !CONFIG_GENERIC_BUG */ 694 694 695 - static inline int module_bug_finalize(const Elf_Ehdr *hdr, 695 + static inline void module_bug_finalize(const Elf_Ehdr *hdr, 696 696 const Elf_Shdr *sechdrs, 697 697 struct module *mod) 698 698 { 699 - return 0; 700 699 } 701 700 static inline void module_bug_cleanup(struct module *mod) {} 702 701 #endif /* CONFIG_GENERIC_BUG */
+4
kernel/module.c
··· 1537 1537 { 1538 1538 struct module *mod = _mod; 1539 1539 list_del(&mod->list); 1540 + module_bug_cleanup(mod); 1540 1541 return 0; 1541 1542 } 1542 1543 ··· 2626 2625 if (err < 0) 2627 2626 goto ddebug; 2628 2627 2628 + module_bug_finalize(info.hdr, info.sechdrs, mod); 2629 2629 list_add_rcu(&mod->list, &modules); 2630 2630 mutex_unlock(&module_mutex); 2631 2631 ··· 2652 2650 mutex_lock(&module_mutex); 2653 2651 /* Unlink carefully: kallsyms could be walking list. */ 2654 2652 list_del_rcu(&mod->list); 2653 + module_bug_cleanup(mod); 2654 + 2655 2655 ddebug: 2656 2656 if (!mod->taints) 2657 2657 dynamic_debug_remove(info.debug);
+2 -4
lib/bug.c
··· 72 72 return NULL; 73 73 } 74 74 75 - int module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs, 76 - struct module *mod) 75 + void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs, 76 + struct module *mod) 77 77 { 78 78 char *secstrings; 79 79 unsigned int i; ··· 97 97 * could potentially lead to deadlock and thus be counter-productive. 98 98 */ 99 99 list_add(&mod->bug_list, &module_bug_list); 100 - 101 - return 0; 102 100 } 103 101 104 102 void module_bug_cleanup(struct module *mod)