Re: [RFC][PATCH 1/2 v2] ftrace/x86: Allow for arguments to be passed in to REGS by default

[Steven Rostedt]

On Wed, 28 Oct 2020 16:36:26 -0400
Steven Rostedt <rost...@goodmis.org> wrote:

> Here's a proof of concept patch. It passes all the kprobe self tests in
> the ftracetest suite. This is just a proof of concept, and I already
> know of a couple of subtle bugs that are easy to fix. But this shows
> the general idea.

And this works for live patching as well (the below patch passed the
live patching test).

To make sure this really worked, I made the regular trampoline put RIP
into the R15 location, and live patching used that.

I'm going to start working on getting rid of FL_SAVE_REGS, by first
changing the regs parameter to struct ftrace_regs * type, and having
all regs users use ftrace_regs_get_regs(), that will return NULL if
it's not full regs (at least for x86), this way we can keep the current
regs (if kprobes *really* needs it). But then having other accesses for
this. And this way, live patching will no longer need a full regs to
work.

Then by default we can have all callbacks have access to the stack and
arguments!

-- Steve

diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
index 1fde1ab6559e..de3bf7ce1c5d 100644
--- a/arch/x86/include/asm/livepatch.h
+++ b/arch/x86/include/asm/livepatch.h
@@ -14,7 +14,7 @@
 
 static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
 {
-       regs->ip = ip;
+       regs->r15 = ip;
 }
 
 #endif /* _ASM_X86_LIVEPATCH_H */
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index f8833fe0ff44..aad647500818 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -148,6 +148,11 @@ SYM_INNER_LABEL(ftrace_caller_op_ptr, SYM_L_GLOBAL)
        /* Load the ftrace_ops into the 3rd parameter */
        movq function_trace_op(%rip), %rdx
 
+       /* put RIP into R15 to test this */
+       movq RIP(%rsp), %rcx
+       movq %rcx, R15(%rsp)
+       movq $0, RIP(%rsp)
+
        /* regs go into 4th parameter */
        leaq (%rsp), %rcx
 
@@ -158,7 +163,7 @@ SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
        call ftrace_stub
 
        /* Handlers can change the RIP */
-       movq RIP(%rsp), %rax
+       movq R15(%rsp), %rax
        movq %rax, MCOUNT_REG_SIZE(%rsp)
 
        restore_mcount_regs
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index c3a5b2675397..3374d53a747a 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -22,6 +22,7 @@ static void arch_ftrace_fill_regs(struct pt_regs *regs,
                                  struct pt_regs *ftrace_regs)
 {
        *regs = *ftrace_regs;
+       regs->ip = regs->r15;
        local_save_flags(regs->flags);
        regs->cs = __KERNEL_CS;
 }
@@ -30,6 +31,7 @@ static void arch_regs_fill_ftrace(struct pt_regs *ftrace_regs,
                                  struct pt_regs *regs)
 {
        *ftrace_regs = *regs;
+       ftrace_regs->r15 = regs->ip;
 }
 
 /* Ftrace callback handler for kprobes -- called under preepmt disabed */
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 6c0164d24bbd..87ecdf486ca3 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -199,8 +199,10 @@ static int klp_patch_func(struct klp_func *func)
                        return -ENOMEM;
 
                ops->fops.func = klp_ftrace_handler;
-               ops->fops.flags = FTRACE_OPS_FL_SAVE_REGS |
-                                 FTRACE_OPS_FL_DYNAMIC |
+               ops->fops.flags = FTRACE_OPS_FL_DYNAMIC |
+#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
+                                 FTRACE_OPS_FL_SAVE_REGS |
+#endif
                                  FTRACE_OPS_FL_IPMODIFY |
                                  FTRACE_OPS_FL_PERMANENT;