Messages in this thread Patch in this message | | | Date | Thu, 30 Apr 2020 23:47:33 -0500 | From | Josh Poimboeuf <> | Subject | Re: [RFC][PATCH] x86/ftrace: Have ftrace trampolines turn read-only at the end of system boot up |
| |
On Thu, Apr 30, 2020 at 08:21:47PM -0400, Steven Rostedt wrote: > The cause is the "ftrace=function" would register the function tracer > and create a trampoline, and it will set it as executable and > read-only. Then the "trace_options=func_stack_trace" would then update > the same trampoline to include the stack tracer version of the function > tracer. But since the trampoline already exists, it updates it with > text_poke_bp(). The problem is that text_poke_bp() called while > system_state == SYSTEM_BOOTING, it will simply do a memcpy() and not > the page mapping, as it would think that the text is still read-write. > But in this case it is not, and we take a fault and crash. > > Instead, lets keep the ftrace trampolines read-write during boot up, > and then when the kernel executable text is set to read-only, the > ftrace trampolines get set to read-only as well.
Would it be easier to just call a new __text_poke_bp() which skips the SYSTEM_BOOTING check, since you know the trampoline will always be read-only?
Like:
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h index 67315fa3956a..710106256916 100644 --- a/arch/x86/include/asm/text-patching.h +++ b/arch/x86/include/asm/text-patching.h @@ -45,6 +45,7 @@ extern void *text_poke(void *addr, const void *opcode, size_t len); extern void text_poke_sync(void); extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len); extern int poke_int3_handler(struct pt_regs *regs); +extern void __text_poke_bp(void *addr, const void *opcode, size_t len, const void *emulate); extern void text_poke_bp(void *addr, const void *opcode, size_t len, const void *emulate); extern void text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate); diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 7867dfb3963e..9cc983cc9291 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -1265,6 +1265,14 @@ void __ref text_poke_queue(void *addr, const void *opcode, size_t len, const voi text_poke_loc_init(tp, addr, opcode, len, emulate); } +void __ref __text_poke_bp(void *addr, const void *opcode, size_t len, const void *emulate) +{ + struct text_poke_loc tp; + + text_poke_loc_init(&tp, addr, opcode, len, emulate); + text_poke_bp_batch(&tp, 1); +} + /** * text_poke_bp() -- update instructions on live kernel on SMP * @addr: address to patch @@ -1278,13 +1286,10 @@ void __ref text_poke_queue(void *addr, const void *opcode, size_t len, const voi */ void __ref text_poke_bp(void *addr, const void *opcode, size_t len, const void *emulate) { - struct text_poke_loc tp; - if (unlikely(system_state == SYSTEM_BOOTING)) { text_poke_early(addr, opcode, len); return; } - text_poke_loc_init(&tp, addr, opcode, len, emulate); - text_poke_bp_batch(&tp, 1); + __text_poke_bp(addr, opcode, len, emulate); } diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 867c126ddabe..c36f51f01f6e 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -469,7 +469,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops) mutex_lock(&text_mutex); /* Do a safe modify in case the trampoline is executing */ new = ftrace_call_replace(ip, (unsigned long)func); - text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL); + __text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL); mutex_unlock(&text_mutex); }
| |