From 285a54efe3861976af9d15e85ff8c91a78d1407b Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Wed, 27 Nov 2019 14:56:52 +0900 Subject: [PATCH] x86/alternatives: Sync bp_patching update for avoiding NULL pointer exception ftracetest multiple_kprobes.tc testcase hits the following NULL pointer exception: BUG: kernel NULL pointer dereference, address: 0000000000000000 PGD 800000007bf60067 P4D 800000007bf60067 PUD 7bf5f067 PMD 0 Oops: 0000 [#1] PREEMPT SMP PTI RIP: 0010:poke_int3_handler+0x39/0x100 Call Trace: do_int3+0xd/0xf0 int3+0x42/0x50 RIP: 0010:sched_clock+0x6/0x10 poke_int3_handler+0x39 was alternatives:958: static inline void *text_poke_addr(struct text_poke_loc *tp) { return _stext + tp->rel_addr; <------ Here is line #958 } This seems to be caused by tp (bp_patching.vec) being NULL but bp_patching.nr_entries != 0. There is a small chance for this to happen, because we have no synchronization between the zeroing of bp_patching.nr_entries and before clearing bp_patching.vec. Steve suggested we could fix this by adding sync_core(), because int3 is done with interrupts disabled, and the on_each_cpu() requires all CPUs to have had their interrupts enabled. [ mingo: Edited the comments and the changelog. ] Suggested-by: Steven Rostedt (VMware) Tested-by: Alexei Starovoitov Signed-off-by: Masami Hiramatsu Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: bristot@redhat.com Fixes: c0213b0ac03c ("x86/alternative: Batch of patch operations") Link: https://lkml.kernel.org/r/157483421229.25881.15314414408559963162.stgit@devnote2 Signed-off-by: Ingo Molnar --- arch/x86/kernel/alternative.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 4552795a8df4..30e86730655c 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -1134,8 +1134,14 @@ static void text_poke_bp_batch(struct text_poke_loc *tp, unsigned int nr_entries * sync_core() implies an smp_mb() and orders this store against * the writing of the new instruction. */ - bp_patching.vec = NULL; bp_patching.nr_entries = 0; + /* + * This sync_core () call ensures that all INT3 handlers in progress + * have finished. This allows poke_int3_handler() after this to + * avoid touching bp_paching.vec by checking nr_entries == 0. + */ + text_poke_sync(); + bp_patching.vec = NULL; } void text_poke_loc_init(struct text_poke_loc *tp, void *addr, -- 2.11.0