diff mbox series

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

Message ID 20190507174400.219947724@goodmis.org (mailing list archive)
State New
Headers show
Series x86_64/ftrace: Emulate calls from int3 when patching functions | expand

Commit Message

Steven Rostedt May 7, 2019, 5:42 p.m. UTC 
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(+)

Comments

Josh Poimboeuf May 7, 2019, 5:53 p.m. UTC | #1 
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.
Steven Rostedt May 7, 2019, 7:01 p.m. UTC | #2 
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 */
Josh Poimboeuf May 7, 2019, 7:14 p.m. UTC | #3 
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.
Steven Rostedt May 7, 2019, 7:20 p.m. UTC | #4 
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
Josh Poimboeuf May 7, 2019, 7:49 p.m. UTC | #5 
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
Steven Rostedt May 7, 2019, 7:58 p.m. UTC | #6 
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
Josh Poimboeuf May 7, 2019, 8:02 p.m. UTC | #7 
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 :-)
diff mbox series

Patch

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 */