[PATCH 4/4] kprobes, x86/alternatives: preempt_disable() when check smp_alt_modules

From: Zhou Chengming
Date: Sat Oct 28 2017 - 08:48:36 EST


Fixes: 2cfa197 "ftrace/alternatives: Introducing *_text_reserved
functions"

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.

Another simpler solution reuse the text_mutex to protect smp_alt_modules
list, all the arch dependent checks of kprobes are inside the
text_mutex, so it's safe. But it will use one text_mutex to protect
2 resources, which is not a good idea. And the original patch and
related discussions are here:
https://patchwork.kernel.org/patch/10029465/

This patchset take a different way to solve the problem. We could
remove the smp_alt mutex, only use preempt_disable() to protect
the smp_alt_modules list, since the list only be used when UP now.
So alternatives_text_reserved() also only need to preempt_disable()
when do the smp_alt_modules list check, it's safe now.

Signed-off-by: Zhou Chengming <zhouchengming1@xxxxxxxxxx>
---
arch/x86/kernel/alternative.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 7eab6f6..b278cad 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -575,6 +575,9 @@ int alternatives_text_reserved(void *start, void *end)
const s32 *poff;
u8 *text_start = start;
u8 *text_end = end;
+ int ret = 0;
+
+ preempt_disable();

list_for_each_entry(mod, &smp_alt_modules, next) {
if (mod->text > text_end || mod->text_end < text_start)
@@ -582,12 +585,16 @@ int alternatives_text_reserved(void *start, void *end)
for (poff = mod->locks; poff < mod->locks_end; poff++) {
const u8 *ptr = (const u8 *)poff + *poff;

- if (text_start <= ptr && text_end > ptr)
- return 1;
+ if (text_start <= ptr && text_end > ptr) {
+ ret = 1;
+ goto out;
+ }
}
}

- return 0;
+out:
+ preempt_enable();
+ return ret;
}
#endif /* CONFIG_SMP */

--
1.8.3.1