Ingo Molnar wrote: > * Alexey Dobriyan wrote: > >> [cc'ing folks whose proc files are affected] >> >> kallsyms_lookup() can call module_address_lookup() which iterates over >> modules list without module_mutex taken. Comment at the top of >> module_address_lookup() says it's for oops resolution so races are >> irrelevant, but in some cases it's reachable from regular code: > > looking at the problem from another angle: wouldnt this be something > that would benefit from freeze_processes()/unfreeze_processes(), and > hence no locking would be required? I also considered this, but it seemed a little too "blunt" to stop everything (including completely unrelated processes and kernel threads) just to remove a module. How about something like this instead? (just for review) It's a little more intrusive, since the interface for symbol lookup is changed (and it affects the callers), but it makes the caller aware that it should set "safe_to_lock" if possible. This way we avoid exposing module_mutex outside of module.c and avoid returning any data pointer to some data structure that might disappear before the caller tries to use it. Some of these changes are actually cleanups, because the callers where creating a dummy modname variables that they didn't use afterwards. The thing I like the less about this patch is the increase of stack usage on some code paths by about 60 bytes. Anyway, I don't have very strong feelings about this, so if you think it would be better to use freeze_processes()/unfreeze_processes(), I can cook up a patch for that too... -- Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes." diffstat: > arch/parisc/kernel/unwind.c | 3 -- > arch/powerpc/xmon/xmon.c | 11 ++++----- > arch/ppc/xmon/xmon.c | 8 +++--- > arch/sh64/kernel/unwind.c | 4 +-- > arch/x86_64/kernel/traps.c | 10 ++++---- > fs/proc/base.c | 3 -- > include/linux/kallsyms.h | 6 +++- > include/linux/module.h | 44 +++++++++++++++++++----------------- > kernel/kallsyms.c | 53 +++++++++++++++++++++----------------------- > kernel/kprobes.c | 6 ++-- > kernel/lockdep.c | 3 -- > kernel/module.c | 44 +++++++++++++++++++++++++++--------- > kernel/time/timer_list.c | 3 -- > kernel/time/timer_stats.c | 3 -- > mm/slab.c | 6 ++-- > 15 files changed, 114 insertions(+), 93 deletions(-)