Re: [for-next][PATCH 04/18] x86/ftrace: Do not jump to direct code in created trampolines

Peter Zijlstra
2020-07-03T08:10:14Z

On Thu, Jul 02, 2020 at 05:58:16PM -0400, Steven Rostedt wrote:

> + /* No need to test direct calls on created trampolines */
> + if (ops->flags & FTRACE_OPS_FL_SAVE_REGS) {
> + /* NOP the jnz 1f; but make sure it's a 2 byte jnz */
> + ip = trampoline + (jmp_offset - start_offset);
> + if (WARN_ON(*(char *)ip != 0x75))
> + goto fail;
> + ret = copy_from_kernel_nofault(ip, ideal_nops[2], 2);

I really don't get this paranoia, what's wrong with memcpy() ?

> + if (ret < 0)
> + goto fail;
> + }

How about something like this?

--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -359,17 +359,11 @@ create_trampoline(struct ftrace_ops *ops
npages = DIV_ROUND_UP(*tramp_size, PAGE_SIZE);

/* Copy ftrace_caller onto the trampoline memory */
- ret = copy_from_kernel_nofault(trampoline, (void *)start_offset, size);
- if (WARN_ON(ret < 0))
- goto fail;
-
- ip = trampoline + size;
+ memcpy(trampoline, (void *)start_offset, size);

/* The trampoline ends with ret(q) */
- retq = (unsigned long)ftrace_stub;
- ret = copy_from_kernel_nofault(ip, (void *)retq, RET_SIZE);
- if (WARN_ON(ret < 0))
- goto fail;
+ ip = trampoline + size;
+ memcpy(ip, text_gen_insn(RET_INSN_OPCODE, NULL, NULL), RET_INSN_SIZE);

/* No need to test direct calls on created trampolines */
if (ops->flags & FTRACE_OPS_FL_SAVE_REGS) {
@@ -377,9 +371,7 @@ create_trampoline(struct ftrace_ops *ops
ip = trampoline + (jmp_offset - start_offset);
if (WARN_ON(*(char *)ip != 0x75))
goto fail;
- ret = copy_from_kernel_nofault(ip, ideal_nops[2], 2);
- if (ret < 0)
- goto fail;
+ memcpy(ip, ideal_nops[2], 2);
}

/*