Skip to content

Commit 5336377

Browse files
committed
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 <[email protected]> Cc: Rusty Russell <[email protected]> Cc: Adrian Bunk <[email protected]> Cc: Andrew Morton <[email protected]> Cc: [email protected] Signed-off-by: Linus Torvalds <[email protected]>
1 parent 2f6b3aa commit 5336377

File tree

11 files changed

+14
-26
lines changed

11 files changed

+14
-26
lines changed

arch/avr32/kernel/module.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,9 @@ int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
314314
vfree(module->arch.syminfo);
315315
module->arch.syminfo = NULL;
316316

317-
return module_bug_finalize(hdr, sechdrs, module);
317+
return 0;
318318
}
319319

320320
void module_arch_cleanup(struct module *module)
321321
{
322-
module_bug_cleanup(module);
323322
}

arch/h8300/kernel/module.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,9 @@ int module_finalize(const Elf_Ehdr *hdr,
112112
const Elf_Shdr *sechdrs,
113113
struct module *me)
114114
{
115-
return module_bug_finalize(hdr, sechdrs, me);
115+
return 0;
116116
}
117117

118118
void module_arch_cleanup(struct module *mod)
119119
{
120-
module_bug_cleanup(mod);
121120
}

arch/mn10300/kernel/module.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -206,13 +206,12 @@ int module_finalize(const Elf_Ehdr *hdr,
206206
const Elf_Shdr *sechdrs,
207207
struct module *me)
208208
{
209-
return module_bug_finalize(hdr, sechdrs, me);
209+
return 0;
210210
}
211211

212212
/*
213213
* finish clearing the module
214214
*/
215215
void module_arch_cleanup(struct module *mod)
216216
{
217-
module_bug_cleanup(mod);
218217
}

arch/parisc/kernel/module.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -941,11 +941,10 @@ int module_finalize(const Elf_Ehdr *hdr,
941941
nsyms = newptr - (Elf_Sym *)symhdr->sh_addr;
942942
DEBUGP("NEW num_symtab %lu\n", nsyms);
943943
symhdr->sh_size = nsyms * sizeof(Elf_Sym);
944-
return module_bug_finalize(hdr, sechdrs, me);
944+
return 0;
945945
}
946946

947947
void module_arch_cleanup(struct module *mod)
948948
{
949949
deregister_unwind_table(mod);
950-
module_bug_cleanup(mod);
951950
}

arch/powerpc/kernel/module.c

-5
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,6 @@ int module_finalize(const Elf_Ehdr *hdr,
6565
const Elf_Shdr *sect;
6666
int err;
6767

68-
err = module_bug_finalize(hdr, sechdrs, me);
69-
if (err)
70-
return err;
71-
7268
/* Apply feature fixups */
7369
sect = find_section(hdr, sechdrs, "__ftr_fixup");
7470
if (sect != NULL)
@@ -101,5 +97,4 @@ int module_finalize(const Elf_Ehdr *hdr,
10197

10298
void module_arch_cleanup(struct module *mod)
10399
{
104-
module_bug_cleanup(mod);
105100
}

arch/s390/kernel/module.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -407,10 +407,9 @@ int module_finalize(const Elf_Ehdr *hdr,
407407
{
408408
vfree(me->arch.syminfo);
409409
me->arch.syminfo = NULL;
410-
return module_bug_finalize(hdr, sechdrs, me);
410+
return 0;
411411
}
412412

413413
void module_arch_cleanup(struct module *mod)
414414
{
415-
module_bug_cleanup(mod);
416415
}

arch/sh/kernel/module.c

-2
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,11 @@ int module_finalize(const Elf_Ehdr *hdr,
149149
int ret = 0;
150150

151151
ret |= module_dwarf_finalize(hdr, sechdrs, me);
152-
ret |= module_bug_finalize(hdr, sechdrs, me);
153152

154153
return ret;
155154
}
156155

157156
void module_arch_cleanup(struct module *mod)
158157
{
159-
module_bug_cleanup(mod);
160158
module_dwarf_cleanup(mod);
161159
}

arch/x86/kernel/module.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,10 @@ int module_finalize(const Elf_Ehdr *hdr,
239239
apply_paravirt(pseg, pseg + para->sh_size);
240240
}
241241

242-
return module_bug_finalize(hdr, sechdrs, me);
242+
return 0;
243243
}
244244

245245
void module_arch_cleanup(struct module *mod)
246246
{
247247
alternatives_smp_module_del(mod);
248-
module_bug_cleanup(mod);
249248
}

include/linux/module.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -686,17 +686,16 @@ extern int module_sysfs_initialized;
686686

687687

688688
#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 *,
690690
struct module *);
691691
void module_bug_cleanup(struct module *);
692692

693693
#else /* !CONFIG_GENERIC_BUG */
694694

695-
static inline int module_bug_finalize(const Elf_Ehdr *hdr,
695+
static inline void module_bug_finalize(const Elf_Ehdr *hdr,
696696
const Elf_Shdr *sechdrs,
697697
struct module *mod)
698698
{
699-
return 0;
700699
}
701700
static inline void module_bug_cleanup(struct module *mod) {}
702701
#endif /* CONFIG_GENERIC_BUG */

kernel/module.c

+4
Original file line numberDiff line numberDiff line change
@@ -1537,6 +1537,7 @@ static int __unlink_module(void *_mod)
15371537
{
15381538
struct module *mod = _mod;
15391539
list_del(&mod->list);
1540+
module_bug_cleanup(mod);
15401541
return 0;
15411542
}
15421543

@@ -2625,6 +2626,7 @@ static struct module *load_module(void __user *umod,
26252626
if (err < 0)
26262627
goto ddebug;
26272628

2629+
module_bug_finalize(info.hdr, info.sechdrs, mod);
26282630
list_add_rcu(&mod->list, &modules);
26292631
mutex_unlock(&module_mutex);
26302632

@@ -2650,6 +2652,8 @@ static struct module *load_module(void __user *umod,
26502652
mutex_lock(&module_mutex);
26512653
/* Unlink carefully: kallsyms could be walking list. */
26522654
list_del_rcu(&mod->list);
2655+
module_bug_cleanup(mod);
2656+
26532657
ddebug:
26542658
if (!mod->taints)
26552659
dynamic_debug_remove(info.debug);

lib/bug.c

+2-4
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ static const struct bug_entry *module_find_bug(unsigned long bugaddr)
7272
return NULL;
7373
}
7474

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)
7777
{
7878
char *secstrings;
7979
unsigned int i;
@@ -97,8 +97,6 @@ int module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
9797
* could potentially lead to deadlock and thus be counter-productive.
9898
*/
9999
list_add(&mod->bug_list, &module_bug_list);
100-
101-
return 0;
102100
}
103101

104102
void module_bug_cleanup(struct module *mod)

0 commit comments

Comments
 (0)