3.16.54-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Zhou Chengming zhouchengming1@huawei.com
commit e846d13958066828a9483d862cc8370a72fadbb6 upstream.
We use alternatives_text_reserved() to check if the address is in the fixed pieces of alternative reserved, but the problem is that we don't hold the smp_alt mutex when call this function. So the list traversal may encounter a deleted list_head if another path is doing alternatives_smp_module_del().
One solution is that we can hold smp_alt mutex before call this function, but the difficult point is that the callers of this functions, arch_prepare_kprobe() and arch_prepare_optimized_kprobe(), are called inside the text_mutex. So we must hold smp_alt mutex before we go into these arch dependent code. But we can't now, the smp_alt mutex is the arch dependent part, only x86 has it. Maybe we can export another arch dependent callback to solve this.
But there is a simpler way to handle this problem. We can reuse the text_mutex to protect smp_alt_modules instead of using another mutex. And all the arch dependent checks of kprobes are inside the text_mutex, so it's safe now.
Signed-off-by: Zhou Chengming zhouchengming1@huawei.com Reviewed-by: Masami Hiramatsu mhiramat@kernel.org Acked-by: Steven Rostedt (VMware) rostedt@goodmis.org Cc: Andy Lutomirski luto@kernel.org Cc: Borislav Petkov bp@alien8.de Cc: Brian Gerst brgerst@gmail.com Cc: Denys Vlasenko dvlasenk@redhat.com Cc: H. Peter Anvin hpa@zytor.com Cc: Josh Poimboeuf jpoimboe@redhat.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: bp@suse.de Fixes: 2cfa197 "ftrace/alternatives: Introducing *_text_reserved functions" Link: http://lkml.kernel.org/r/1509585501-79466-1-git-send-email-zhouchengming1@hu... Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Ben Hutchings ben@decadent.org.uk --- arch/x86/kernel/alternative.c | 26 +++++++++++++------------- kernel/extable.c | 2 ++ 2 files changed, 15 insertions(+), 13 deletions(-)
--- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -409,7 +409,6 @@ static void alternatives_smp_lock(const { const s32 *poff;
- mutex_lock(&text_mutex); for (poff = start; poff < end; poff++) { u8 *ptr = (u8 *)poff + *poff;
@@ -419,7 +418,6 @@ static void alternatives_smp_lock(const if (*ptr == 0x3e) text_poke(ptr, ((unsigned char []){0xf0}), 1); } - mutex_unlock(&text_mutex); }
static void alternatives_smp_unlock(const s32 *start, const s32 *end, @@ -427,7 +425,6 @@ static void alternatives_smp_unlock(cons { const s32 *poff;
- mutex_lock(&text_mutex); for (poff = start; poff < end; poff++) { u8 *ptr = (u8 *)poff + *poff;
@@ -437,7 +434,6 @@ static void alternatives_smp_unlock(cons if (*ptr == 0xf0) text_poke(ptr, ((unsigned char []){0x3E}), 1); } - mutex_unlock(&text_mutex); }
struct smp_alt_module { @@ -456,8 +452,7 @@ struct smp_alt_module { struct list_head next; }; static LIST_HEAD(smp_alt_modules); -static DEFINE_MUTEX(smp_alt); -static bool uniproc_patched = false; /* protected by smp_alt */ +static bool uniproc_patched = false; /* protected by text_mutex */
void __init_or_module alternatives_smp_module_add(struct module *mod, char *name, @@ -466,7 +461,7 @@ void __init_or_module alternatives_smp_m { struct smp_alt_module *smp;
- mutex_lock(&smp_alt); + mutex_lock(&text_mutex); if (!uniproc_patched) goto unlock;
@@ -493,14 +488,14 @@ void __init_or_module alternatives_smp_m smp_unlock: alternatives_smp_unlock(locks, locks_end, text, text_end); unlock: - mutex_unlock(&smp_alt); + mutex_unlock(&text_mutex); }
void __init_or_module alternatives_smp_module_del(struct module *mod) { struct smp_alt_module *item;
- mutex_lock(&smp_alt); + mutex_lock(&text_mutex); list_for_each_entry(item, &smp_alt_modules, next) { if (mod != item->mod) continue; @@ -508,7 +503,7 @@ void __init_or_module alternatives_smp_m kfree(item); break; } - mutex_unlock(&smp_alt); + mutex_unlock(&text_mutex); }
void alternatives_enable_smp(void) @@ -518,7 +513,7 @@ void alternatives_enable_smp(void) /* Why bother if there are no other CPUs? */ BUG_ON(num_possible_cpus() == 1);
- mutex_lock(&smp_alt); + mutex_lock(&text_mutex);
if (uniproc_patched) { pr_info("switching to SMP code\n"); @@ -530,10 +525,13 @@ void alternatives_enable_smp(void) mod->text, mod->text_end); uniproc_patched = false; } - mutex_unlock(&smp_alt); + mutex_unlock(&text_mutex); }
-/* Return 1 if the address range is reserved for smp-alternatives */ +/* + * Return 1 if the address range is reserved for SMP-alternatives. + * Must hold text_mutex. + */ int alternatives_text_reserved(void *start, void *end) { struct smp_alt_module *mod; @@ -541,6 +539,8 @@ int alternatives_text_reserved(void *sta u8 *text_start = start; u8 *text_end = end;
+ lockdep_assert_held(&text_mutex); + list_for_each_entry(mod, &smp_alt_modules, next) { if (mod->text > text_end || mod->text_end < text_start) continue; --- a/kernel/extable.c +++ b/kernel/extable.c @@ -28,6 +28,8 @@ * mutex protecting text section modification (dynamic code patching). * some users need to sleep (allocating memory...) while they hold this lock. * + * Note: Also protects SMP-alternatives modification on x86. + * * NOT exported to modules - patching kernel text is a really delicate matter. */ DEFINE_MUTEX(text_mutex);