Messages in this thread | | | Date | Wed, 24 May 2017 23:33:36 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| |
On Wed, 24 May 2017 15:47:17 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote:
> ftrace uses module_alloc() to allocate trampoline pages. The mapping of > module_alloc() is RWX, which makes sense as the memory is written to right > after allocation. But nothing makes these pages RO after writing to them. > > This problem exists since ftrace uses trampolines on x86, but it went > unnoticed because the W=X sanity check only triggers when the tracer > builtin selftests are enabled. Though the mappings are also created W+X w/o > the self tests when the tracer is used after booting. > > Add proper set_memory_rw/ro() calls to [un]protect the trampolines before > and after modification. >
This looks good to me. (same as I did for kprobes trampoline pages)
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you,
> Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for ftrace_ops") > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/ftrace.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc > unsigned long offset; > unsigned long ip; > unsigned int size; > - int ret; > + int ret, npages; > > if (ops->trampoline) { > /* > @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc > */ > if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) > return; > + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; > + set_memory_rw(ops->trampoline, npages); > } else { > ops->trampoline = create_trampoline(ops, &size); > if (!ops->trampoline) > return; > ops->trampoline_size = size; > + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > } > > offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); > @@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc > /* Do a safe modify in case the trampoline is executing */ > new = ftrace_call_replace(ip, (unsigned long)func); > ret = update_ftrace_func(ip, new); > + set_memory_ro(ops->trampoline, npages); > > /* The update should never fail */ > WARN_ON(ret);
-- Masami Hiramatsu <mhiramat@kernel.org>
| |