[RFC][PATCH 0/3] x86_64/ftrace: Emulate calls from int3 when patching functions

[RFC][PATCH 0/3] x86_64/ftrace: Emulate calls from int3 when patching functions

[Steven Rostedt]

Nicolai Stange discovered that Live Kernel Patching can have unforseen
consequences if tracing is enabled when there are functions that are
patched. The reason being, is that Live Kernel patching is built on top
of ftrace, which will have the patched functions call the live kernel
trampoline directly, and that trampoline will modify the regs->ip address
to return to the patched function.

But in the transition between changing the call to the customized
trampoline, the tracing code is needed to have its handler called
an well, so the function fentry location must be changed from calling
the live kernel patching trampoline, to the ftrace_reg_caller trampoline
which will iterate through all the registered ftrace handlers for
that function.

During this transition, a break point is added to do the live code
modifications. But if that break point is hit, it just skips calling
any handler, and makes the call site act as a nop. For tracing, the
worse that can happen is that you miss a function being traced, but
for live kernel patching the affects are more severe, as the old buggy
function is now called.

To solve this, an int3_emulate_call() is created for x86_64 to allow
ftrace on x86_64 to emulate the call to ftrace_regs_caller() which will
make sure all the registered handlers to that function are still called.
And this keeps live kernel patching happy!

To mimimize the changes, and to avoid controversial patches, this
only changes x86_64. Due to the way x86_32 implements the regs->sp
the complexity of emulating calls on that platform is too much for
stable patches, and live kernel patching does not support x86_32 anyway.

Josh Poimboeuf (1):
      x86_64: Add gap to int3 to allow for call emulation

Peter Zijlstra (2):
      x86_64: Allow breakpoints to emulate call functions
      ftrace/x86_64: Emulate call function while updating in breakpoint handler

----
 arch/x86/entry/entry_64.S            | 18 ++++++++++++++++--
 arch/x86/include/asm/text-patching.h | 22 ++++++++++++++++++++++
 arch/x86/kernel/ftrace.c             | 32 +++++++++++++++++++++++++++-----
 3 files changed, 65 insertions(+), 7 deletions(-)

[RFC][PATCH 1/3] x86_64: Add gap to int3 to allow for call emulation

[Steven Rostedt]

From: Josh Poimboeuf <jpoimboe@redhat.com>

To allow an int3 handler to emulate a call instruction, it must be able to
push a return address onto the stack. Add a gap to the stack to allow the
int3 handler to push the return address and change the return from int3 to
jump straight to the emulated called function target.

Link: http://lkml.kernel.org/r/20181130183917.hxmti5josgq4clti@treble
Link: http://lkml.kernel.org/r/20190502162133.GX2623@hirez.programming.kicks-ass.net

[
  Note, this is needed to allow Live Kernel Patching to not miss calling a
  patched function when tracing is enabled. -- Steven Rostedt
]

Cc: stable@vger.kernel.org
Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/entry/entry_64.S | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 1f0efdb7b629..00df6b135ab1 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -879,7 +879,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
  * @paranoid == 2 is special: the stub will never switch stacks.  This is for
  * #DF: if the thread stack is somehow unusable, we'll still get a useful OOPS.
  */
-.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
+.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1 create_gap=0
 ENTRY(\sym)
 	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
 
@@ -899,6 +899,20 @@ ENTRY(\sym)
 	jnz	.Lfrom_usermode_switch_stack_\@
 	.endif
 
+	.if \create_gap == 1
+	/*
+	 * If coming from kernel space, create a 6-word gap to allow the static
+	 * call #BP handler to emulate a call instruction.
+	 */
+	testb	$3, CS-ORIG_RAX(%rsp)
+	jnz	.Lfrom_usermode_no_gap_\@
+	.rept	6
+	pushq	5*8(%rsp)
+	.endr
+	UNWIND_HINT_IRET_REGS offset=8
+.Lfrom_usermode_no_gap_\@:
+	.endif
+
 	.if \paranoid
 	call	paranoid_entry
 	.else
@@ -1130,7 +1144,7 @@ apicinterrupt3 HYPERV_STIMER0_VECTOR \
 #endif /* CONFIG_HYPERV */
 
 idtentry debug			do_debug		has_error_code=0	paranoid=1 shift_ist=DEBUG_STACK
-idtentry int3			do_int3			has_error_code=0
+idtentry int3			do_int3			has_error_code=0	create_gap=1
 idtentry stack_segment		do_stack_segment	has_error_code=1
 
 #ifdef CONFIG_XEN_PV
-- 
2.20.1

[RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Steven Rostedt]

From: Peter Zijlstra <peterz@infradead.org>

In order to allow breakpoints to emulate call functions, they need to push
the return address onto the stack. But because the breakpoint exception
frame is added to the stack when the breakpoint is hit, there's no room to
add the address onto the stack and return to the address of the emulated
called funtion.

To handle this, copy the exception frame on entry of the breakpoint handler
and have leave a gap that can be used to add a return address to the stack
frame and return from the breakpoint to the emulated called function,
allowing for that called function to return back to the location after the
breakpoint was placed.

The helper functions were also added:

  int3_emulate_push(): to push the address onto the gap in the stack
  int3_emulate_jmp(): changes the location of the regs->ip to return there.
  int3_emulate_call(): push the return address and change regs->ip

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Nicolai Stange <nstange@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: the arch/x86 maintainers <x86@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org>
Cc: stable@vger.kernel.org
Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
[ Modified to only work for x86_64 ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/include/asm/text-patching.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index e85ff65c43c3..455bf9f88233 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -39,4 +39,26 @@ extern int poke_int3_handler(struct pt_regs *regs);
 extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
 extern int after_bootmem;
 
+static inline void int3_emulate_jmp(struct pt_regs *regs, unsigned long ip)
+{
+	regs->ip = ip;
+}
+
+#define INT3_INSN_SIZE 1
+#define CALL_INSN_SIZE 5
+
+#ifdef CONFIG_X86_64
+static inline void int3_emulate_push(struct pt_regs *regs, unsigned long val)
+{
+	regs->sp -= sizeof(unsigned long);
+	*(unsigned long *)regs->sp = val;
+}
+
+static inline void int3_emulate_call(struct pt_regs *regs, unsigned long func)
+{
+	int3_emulate_push(regs, regs->ip - INT3_INSN_SIZE + CALL_INSN_SIZE);
+	int3_emulate_jmp(regs, func);
+}
+#endif
+
 #endif /* _ASM_X86_TEXT_PATCHING_H */
-- 
2.20.1

[RFC][PATCH 3/3] ftrace/x86_64: Emulate call function while updating in breakpoint handler

[Steven Rostedt]

From: Peter Zijlstra <peterz@infradead.org>

Nicolai Stange discovered[1] that if live kernel patching is enabled, and the
function tracer started tracing the same function that was patched, the
conversion of the fentry call site during the translation of going from
calling the live kernel patch trampoline to the iterator trampoline, would
have as slight window where it didn't call anything.

As live kernel patching depends on ftrace to always call its code (to
prevent the function being traced from being called, as it will redirect
it). This small window would allow the old buggy function to be called, and
this can cause undesirable results.

Nicolai submitted new patches[2] but these were controversial. As this is
similar to the static call emulation issues that came up a while ago[3].
But after some debate[4][5] adding a gap in the stack when entering the
breakpoint handler allows for pushing the return address onto the stack to
easily emulate a call.

[1] http://lkml.kernel.org/r/20180726104029.7736-1-nstange@suse.de
[2] http://lkml.kernel.org/r/20190427100639.15074-1-nstange@suse.de
[3] http://lkml.kernel.org/r/3cf04e113d71c9f8e4be95fb84a510f085aa4afa.1541711457.git.jpoimboe@redhat.com
[4] http://lkml.kernel.org/r/CAHk-=wh5OpheSU8Em_Q3Hg8qw_JtoijxOdPtHru6d+5K8TWM=A@mail.gmail.com
[5] http://lkml.kernel.org/r/CAHk-=wjvQxY4DvPrJ6haPgAa6b906h=MwZXO6G8OtiTGe=N7_w@mail.gmail.com

[
  Live kernel patching is not implemented on x86_32, thus the emulate
  calls are only for x86_64.
]

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Nicolai Stange <nstange@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: the arch/x86 maintainers <x86@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org>
Cc: stable@vger.kernel.org
Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
[ Changed to only implement emulated calls for x86_64 ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/kernel/ftrace.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index ef49517f6bb2..bd553b3af22e 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -29,6 +29,7 @@
 #include <asm/kprobes.h>
 #include <asm/ftrace.h>
 #include <asm/nops.h>
+#include <asm/text-patching.h>
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
@@ -231,6 +232,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 }
 
 static unsigned long ftrace_update_func;
+static unsigned long ftrace_update_func_call;
 
 static int update_ftrace_func(unsigned long ip, void *new)
 {
@@ -259,6 +261,8 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	unsigned char *new;
 	int ret;
 
+	ftrace_update_func_call = (unsigned long)func;
+
 	new = ftrace_call_replace(ip, (unsigned long)func);
 	ret = update_ftrace_func(ip, new);
 
@@ -294,13 +298,28 @@ int ftrace_int3_handler(struct pt_regs *regs)
 	if (WARN_ON_ONCE(!regs))
 		return 0;
 
-	ip = regs->ip - 1;
-	if (!ftrace_location(ip) && !is_ftrace_caller(ip))
-		return 0;
+	ip = regs->ip - INT3_INSN_SIZE;
 
-	regs->ip += MCOUNT_INSN_SIZE - 1;
+#ifdef CONFIG_X86_64
+	if (ftrace_location(ip)) {
+		int3_emulate_call(regs, (unsigned long)ftrace_regs_caller);
+		return 1;
+	} else if (is_ftrace_caller(ip)) {
+		if (!ftrace_update_func_call) {
+			int3_emulate_jmp(regs, ip + CALL_INSN_SIZE);
+			return 1;
+		}
+		int3_emulate_call(regs, ftrace_update_func_call);
+		return 1;
+	}
+#else
+	if (ftrace_location(ip) || is_ftrace_caller(ip)) {
+		int3_emulate_jmp(regs, ip + CALL_INSN_SIZE);
+		return 1;
+	}
+#endif
 
-	return 1;
+	return 0;
 }
 NOKPROBE_SYMBOL(ftrace_int3_handler);
 
@@ -859,6 +878,8 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
 
 	func = ftrace_ops_get_func(ops);
 
+	ftrace_update_func_call = (unsigned long)func;
+
 	/* Do a safe modify in case the trampoline is executing */
 	new = ftrace_call_replace(ip, (unsigned long)func);
 	ret = update_ftrace_func(ip, new);
@@ -960,6 +981,7 @@ static int ftrace_mod_jmp(unsigned long ip, void *func)
 {
 	unsigned char *new;
 
+	ftrace_update_func_call = 0UL;
 	new = ftrace_jmp_replace(ip, (unsigned long)func);
 
 	return update_ftrace_func(ip, new);
-- 
2.20.1

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Josh Poimboeuf]

On Tue, May 07, 2019 at 01:42:29PM -0400, Steven Rostedt wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> 
> In order to allow breakpoints to emulate call functions, they need to push
> the return address onto the stack. But because the breakpoint exception
> frame is added to the stack when the breakpoint is hit, there's no room to
> add the address onto the stack and return to the address of the emulated
> called funtion.
> 
> To handle this, copy the exception frame on entry of the breakpoint handler
> and have leave a gap that can be used to add a return address to the stack
> frame and return from the breakpoint to the emulated called function,
> allowing for that called function to return back to the location after the
> breakpoint was placed.

This part is done by patch 1.

> 
> The helper functions were also added:

No longer "also" :-)

>   int3_emulate_push(): to push the address onto the gap in the stack
>   int3_emulate_jmp(): changes the location of the regs->ip to return there.
>   int3_emulate_call(): push the return address and change regs->ip
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Nicolai Stange <nstange@suse.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: the arch/x86 maintainers <x86@kernel.org>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Miroslav Benes <mbenes@suse.cz>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Joe Lawrence <joe.lawrence@redhat.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nayna Jain <nayna@linux.ibm.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joerg Roedel <jroedel@suse.de>
> Cc: "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org>
> Cc: stable@vger.kernel.org
> Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> [ Modified to only work for x86_64 ]
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  arch/x86/include/asm/text-patching.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
> index e85ff65c43c3..455bf9f88233 100644
> --- a/arch/x86/include/asm/text-patching.h
> +++ b/arch/x86/include/asm/text-patching.h
> @@ -39,4 +39,26 @@ extern int poke_int3_handler(struct pt_regs *regs);
>  extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
>  extern int after_bootmem;
>  
> +static inline void int3_emulate_jmp(struct pt_regs *regs, unsigned long ip)
> +{
> +	regs->ip = ip;
> +}
> +
> +#define INT3_INSN_SIZE 1
> +#define CALL_INSN_SIZE 5
> +
> +#ifdef CONFIG_X86_64
> +static inline void int3_emulate_push(struct pt_regs *regs, unsigned long val)
> +{
> +	regs->sp -= sizeof(unsigned long);
> +	*(unsigned long *)regs->sp = val;
> +}

How this works isn't really obvious.  A comment is probably warranted to
explain the fact that the int3 entry code reserved some space on the
stack.

-- 
Josh

Re: [RFC][PATCH 1/3] x86_64: Add gap to int3 to allow for call emulation

[Josh Poimboeuf]

On Tue, May 07, 2019 at 01:42:28PM -0400, Steven Rostedt wrote:
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> To allow an int3 handler to emulate a call instruction, it must be able to
> push a return address onto the stack. Add a gap to the stack to allow the
> int3 handler to push the return address and change the return from int3 to
> jump straight to the emulated called function target.
> 
> Link: http://lkml.kernel.org/r/20181130183917.hxmti5josgq4clti@treble
> Link: http://lkml.kernel.org/r/20190502162133.GX2623@hirez.programming.kicks-ass.net
> 
> [
>   Note, this is needed to allow Live Kernel Patching to not miss calling a
>   patched function when tracing is enabled. -- Steven Rostedt
> ]
> 
> Cc: stable@vger.kernel.org
> Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  arch/x86/entry/entry_64.S | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 1f0efdb7b629..00df6b135ab1 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -879,7 +879,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
>   * @paranoid == 2 is special: the stub will never switch stacks.  This is for
>   * #DF: if the thread stack is somehow unusable, we'll still get a useful OOPS.
>   */
> -.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
> +.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1 create_gap=0
>  ENTRY(\sym)
>  	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
>  
> @@ -899,6 +899,20 @@ ENTRY(\sym)
>  	jnz	.Lfrom_usermode_switch_stack_\@
>  	.endif
>  
> +	.if \create_gap == 1
> +	/*
> +	 * If coming from kernel space, create a 6-word gap to allow the static
> +	 * call #BP handler to emulate a call instruction.

Might as well refer to it as the int3 handler, since that's what the
rest of the code calls it.  Also, no static calls yet :-)  So:

s/static call #BP handler/int3 handler/

-- 
Josh

Re: [RFC][PATCH 1/3] x86_64: Add gap to int3 to allow for call emulation

[Steven Rostedt]

On Tue, 7 May 2019 12:56:55 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:


> > +	.if \create_gap == 1
> > +	/*
> > +	 * If coming from kernel space, create a 6-word gap to allow the static
> > +	 * call #BP handler to emulate a call instruction.  
> 
> Might as well refer to it as the int3 handler, since that's what the
> rest of the code calls it.  Also, no static calls yet :-)  So:
> 
> s/static call #BP handler/int3 handler/
> 

Done.

-- Steve

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Steven Rostedt]

On Tue, 7 May 2019 12:53:42 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> > To handle this, copy the exception frame on entry of the breakpoint handler
> > and have leave a gap that can be used to add a return address to the stack
> > frame and return from the breakpoint to the emulated called function,
> > allowing for that called function to return back to the location after the
> > breakpoint was placed.  
> 
> This part is done by patch 1.
> 
> > 
> > The helper functions were also added:  
> 
> No longer "also" :-)


> > +#ifdef CONFIG_X86_64
> > +static inline void int3_emulate_push(struct pt_regs *regs, unsigned long val)
> > +{
> > +	regs->sp -= sizeof(unsigned long);
> > +	*(unsigned long *)regs->sp = val;
> > +}  
> 
> How this works isn't really obvious.  A comment is probably warranted to
> explain the fact that the int3 entry code reserved some space on the
> stack.
> 


How's this?

-- Steve

From d29dc2e9e0275c9857932b80cebc01551b669efb Mon Sep 17 00:00:00 2001
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed, 1 May 2019 15:11:17 +0200
Subject: [PATCH] x86_64: Allow breakpoints to emulate call functions

In order to allow breakpoints to emulate call functions, they need to push
the return address onto the stack. But because the breakpoint exception
frame is added to the stack when the breakpoint is hit, there's no room to
add the address onto the stack and return to the address of the emulated
called funtion.

This helper functions are added:

  int3_emulate_jmp(): changes the location of the regs->ip to return there.

 (The next two are only for x86_64)
  int3_emulate_push(): to push the address onto the gap in the stack
  int3_emulate_call(): push the return address and change regs->ip

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Nicolai Stange <nstange@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: the arch/x86 maintainers <x86@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org>
Cc: stable@vger.kernel.org
Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
[ Modified to only work for x86_64 and added comment to int3_emulate_push() ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/include/asm/text-patching.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index e85ff65c43c3..05861cc08787 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -39,4 +39,32 @@ extern int poke_int3_handler(struct pt_regs *regs);
 extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
 extern int after_bootmem;
 
+static inline void int3_emulate_jmp(struct pt_regs *regs, unsigned long ip)
+{
+	regs->ip = ip;
+}
+
+#define INT3_INSN_SIZE 1
+#define CALL_INSN_SIZE 5
+
+#ifdef CONFIG_X86_64
+static inline void int3_emulate_push(struct pt_regs *regs, unsigned long val)
+{
+	/*
+	 * The int3 handler in entry_64.S adds a gap between the
+	 * stack where the break point happened, and the saving of
+	 * pt_regs. We can extend the original stack because of
+	 * this gap. See the idtentry macro's create_gap option.
+	 */
+	regs->sp -= sizeof(unsigned long);
+	*(unsigned long *)regs->sp = val;
+}
+
+static inline void int3_emulate_call(struct pt_regs *regs, unsigned long func)
+{
+	int3_emulate_push(regs, regs->ip - INT3_INSN_SIZE + CALL_INSN_SIZE);
+	int3_emulate_jmp(regs, func);
+}
+#endif
+
 #endif /* _ASM_X86_TEXT_PATCHING_H */
-- 
2.20.1

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Josh Poimboeuf]

On Tue, May 07, 2019 at 03:01:53PM -0400, Steven Rostedt wrote:
> How's this?
> 
> -- Steve
> 
> From d29dc2e9e0275c9857932b80cebc01551b669efb Mon Sep 17 00:00:00 2001
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed, 1 May 2019 15:11:17 +0200
> Subject: [PATCH] x86_64: Allow breakpoints to emulate call functions
> 
> In order to allow breakpoints to emulate call functions, they need to push
> the return address onto the stack. But because the breakpoint exception
> frame is added to the stack when the breakpoint is hit, there's no room to
> add the address onto the stack and return to the address of the emulated
> called funtion.

The 2nd sentence can probably be removed since it's technically no
longer true, thanks to the previous patch.

> This helper functions are added:

"These"

> 
>   int3_emulate_jmp(): changes the location of the regs->ip to return there.
> 
>  (The next two are only for x86_64)
>   int3_emulate_push(): to push the address onto the gap in the stack
>   int3_emulate_call(): push the return address and change regs->ip
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Nicolai Stange <nstange@suse.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: the arch/x86 maintainers <x86@kernel.org>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Miroslav Benes <mbenes@suse.cz>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Joe Lawrence <joe.lawrence@redhat.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nayna Jain <nayna@linux.ibm.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joerg Roedel <jroedel@suse.de>
> Cc: "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@vger.kernel.org>
> Cc: stable@vger.kernel.org
> Fixes: b700e7f03df5 ("livepatch: kernel: add support for live patching")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> [ Modified to only work for x86_64 and added comment to int3_emulate_push() ]
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  arch/x86/include/asm/text-patching.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
> index e85ff65c43c3..05861cc08787 100644
> --- a/arch/x86/include/asm/text-patching.h
> +++ b/arch/x86/include/asm/text-patching.h
> @@ -39,4 +39,32 @@ extern int poke_int3_handler(struct pt_regs *regs);
>  extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
>  extern int after_bootmem;
>  
> +static inline void int3_emulate_jmp(struct pt_regs *regs, unsigned long ip)
> +{
> +	regs->ip = ip;
> +}
> +
> +#define INT3_INSN_SIZE 1
> +#define CALL_INSN_SIZE 5
> +
> +#ifdef CONFIG_X86_64
> +static inline void int3_emulate_push(struct pt_regs *regs, unsigned long val)
> +{
> +	/*
> +	 * The int3 handler in entry_64.S adds a gap between the
> +	 * stack where the break point happened, and the saving of
> +	 * pt_regs. We can extend the original stack because of
> +	 * this gap. See the idtentry macro's create_gap option.
> +	 */
> +	regs->sp -= sizeof(unsigned long);
> +	*(unsigned long *)regs->sp = val;

Looks good.

-- 
Josh

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Steven Rostedt]

On Tue, 7 May 2019 14:14:12 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Tue, May 07, 2019 at 03:01:53PM -0400, Steven Rostedt wrote:
> > How's this?
> > 
> > -- Steve
> > 
> > From d29dc2e9e0275c9857932b80cebc01551b669efb Mon Sep 17 00:00:00 2001
> > From: Peter Zijlstra <peterz@infradead.org>
> > Date: Wed, 1 May 2019 15:11:17 +0200
> > Subject: [PATCH] x86_64: Allow breakpoints to emulate call functions
> > 
> > In order to allow breakpoints to emulate call functions, they need to push
> > the return address onto the stack. But because the breakpoint exception
> > frame is added to the stack when the breakpoint is hit, there's no room to
> > add the address onto the stack and return to the address of the emulated
> > called funtion.  
> 
> The 2nd sentence can probably be removed since it's technically no
> longer true, thanks to the previous patch.
> 
> > This helper functions are added:  
> 
> "These"

New version:

    x86_64: Allow breakpoints to emulate call functions
    
    In order to allow breakpoints to emulate call functions, they need to push
    the return address onto the stack. The x86_64 int3 handler adds a small gap
    to allow the stack to grow some. Use this gap to add the return address to
    be able to emulate a call instruction at the breakpoint location.
    
    These helper functions are added:
    
      int3_emulate_jmp(): changes the location of the regs->ip to return there.
    
     (The next two are only for x86_64)
      int3_emulate_push(): to push the address onto the gap in the stack
      int3_emulate_call(): push the return address and change regs->ip

-- Steve

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Josh Poimboeuf]

On Tue, May 07, 2019 at 03:20:16PM -0400, Steven Rostedt wrote:
> On Tue, 7 May 2019 14:14:12 -0500
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > On Tue, May 07, 2019 at 03:01:53PM -0400, Steven Rostedt wrote:
> > > How's this?
> > > 
> > > -- Steve
> > > 
> > > From d29dc2e9e0275c9857932b80cebc01551b669efb Mon Sep 17 00:00:00 2001
> > > From: Peter Zijlstra <peterz@infradead.org>
> > > Date: Wed, 1 May 2019 15:11:17 +0200
> > > Subject: [PATCH] x86_64: Allow breakpoints to emulate call functions
> > > 
> > > In order to allow breakpoints to emulate call functions, they need to push
> > > the return address onto the stack. But because the breakpoint exception
> > > frame is added to the stack when the breakpoint is hit, there's no room to
> > > add the address onto the stack and return to the address of the emulated
> > > called funtion.  
> > 
> > The 2nd sentence can probably be removed since it's technically no
> > longer true, thanks to the previous patch.
> > 
> > > This helper functions are added:  
> > 
> > "These"
> 
> New version:
> 
>     x86_64: Allow breakpoints to emulate call functions
>     
>     In order to allow breakpoints to emulate call functions, they need to push

Sorry to keep nitpicking, but "call functions" -> "function calls" would
sound more accurate to me (in both subject and description).

Otherwise it looks good.

>     the return address onto the stack. The x86_64 int3 handler adds a small gap
>     to allow the stack to grow some. Use this gap to add the return address to
>     be able to emulate a call instruction at the breakpoint location.
>     
>     These helper functions are added:
>     
>       int3_emulate_jmp(): changes the location of the regs->ip to return there.
>     
>      (The next two are only for x86_64)
>       int3_emulate_push(): to push the address onto the gap in the stack
>       int3_emulate_call(): push the return address and change regs->ip

-- 
Josh

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Steven Rostedt]

On Tue, 7 May 2019 14:49:25 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> > New version:
> > 
> >     x86_64: Allow breakpoints to emulate call functions
> >     
> >     In order to allow breakpoints to emulate call functions, they need to push  
> 
> Sorry to keep nitpicking, but "call functions" -> "function calls" would
> sound more accurate to me (in both subject and description).

I disagree ;-)

Matters how you look at it. I look at it as emulating the "call"
function, not a function call. Like emulating an "addl" function, or a
"jmp" function.

See?

To remove the ambiguity, I could replace "function" with "instruction".

> 
> Otherwise it looks good.

Thanks!

-- Steve

Re: [RFC][PATCH 2/3] x86_64: Allow breakpoints to emulate call functions

[Josh Poimboeuf]

On Tue, May 07, 2019 at 03:58:17PM -0400, Steven Rostedt wrote:
> On Tue, 7 May 2019 14:49:25 -0500
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > > New version:
> > > 
> > >     x86_64: Allow breakpoints to emulate call functions
> > >     
> > >     In order to allow breakpoints to emulate call functions, they need to push  
> > 
> > Sorry to keep nitpicking, but "call functions" -> "function calls" would
> > sound more accurate to me (in both subject and description).
> 
> I disagree ;-)
> 
> Matters how you look at it. I look at it as emulating the "call"
> function, not a function call. Like emulating an "addl" function, or a
> "jmp" function.
> 
> See?

I kind of see your point... but then you're overloading the meaning of
the word "function", in a context where it clearly means something else.

> To remove the ambiguity, I could replace "function" with "instruction".

Yes, that would be much better :-)

-- 
Josh