[-tip,v8,10/13] x86/kprobes: Push a fake return address at kretprobe_trampoline

Commit Message

Masami Hiramatsu (Google) June 18, 2021, 7:06 a.m. UTC

This changes x86/kretprobe stack frame on kretprobe_trampoline
a bit, which now push the kretprobe_trampoline as a fake return
address at the bottom of the stack frame. With this fix, the ORC
unwinder will see the kretprobe_trampoline as a return address.

Signed-off-by: Masami Hiramatsu <[e-mail]>
Suggested-by: Josh Poimboeuf <[e-mail]>
Tested-by: Andrii Nakryik <[e-mail]>
Acked-by: Josh Poimboeuf <[e-mail]>
---
 arch/x86/kernel/kprobes/core.c |   31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

Comments

Ingo Molnar July 5, 2021, 8:17 a.m. UTC | #1

* Masami Hiramatsu <[e-mail]> wrote:

> +	/* Replace fake return address with real one. */
> +	*frame_pointer = kretprobe_trampoline_handler(regs, frame_pointer);
> +	/*
> +	 * Move flags to sp so that kretprobe_trapmoline can return
> +	 * right after popf.

What is a trapmoline?

Also, in the x86 code we capitalize register and instruction names so that 
they are more distinctive and easier to read in the flow of English text.

Thanks,

	Ingo

Masami Hiramatsu (Google) July 9, 2021, 2:55 p.m. UTC | #2

On Mon, 5 Jul 2021 10:17:26 +0200
Ingo Molnar <[e-mail]> wrote:

> 
> * Masami Hiramatsu <[e-mail]> wrote:
> 
> > +	/* Replace fake return address with real one. */
> > +	*frame_pointer = kretprobe_trampoline_handler(regs, frame_pointer);
> > +	/*
> > +	 * Move flags to sp so that kretprobe_trapmoline can return
> > +	 * right after popf.
> 
> What is a trapmoline?

This means kretprobe_trampoline() code.

> 
> Also, in the x86 code we capitalize register and instruction names so that 
> they are more distinctive and easier to read in the flow of English text.

OK, let me update it.

Thank you,

> 
> Thanks,
> 
> 	Ingo

Patch

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 74f049b6e77f..4d040aaf969b 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1041,28 +1041,31 @@  asm(
 	".global kretprobe_trampoline\n"
 	".type kretprobe_trampoline, @function\n"
 	"kretprobe_trampoline:\n"
-	/* We don't bother saving the ss register */
 #ifdef CONFIG_X86_64
-	"	pushq %rsp\n"
+	/* Push fake return address to tell the unwinder it's a kretprobe */
+	"	pushq $kretprobe_trampoline\n"
 	UNWIND_HINT_FUNC
+	/* Save the sp-8, this will be fixed later */
+	"	pushq %rsp\n"
 	"	pushfq\n"
 	SAVE_REGS_STRING
 	"	movq %rsp, %rdi\n"
 	"	call trampoline_handler\n"
-	/* Replace saved sp with true return address. */
-	"	movq %rax, 19*8(%rsp)\n"
 	RESTORE_REGS_STRING
+	"	addq $8, %rsp\n"
 	"	popfq\n"
 #else
-	"	pushl %esp\n"
+	/* Push fake return address to tell the unwinder it's a kretprobe */
+	"	pushl $kretprobe_trampoline\n"
 	UNWIND_HINT_FUNC
+	/* Save the sp-4, this will be fixed later */
+	"	pushl %esp\n"
 	"	pushfl\n"
 	SAVE_REGS_STRING
 	"	movl %esp, %eax\n"
 	"	call trampoline_handler\n"
-	/* Replace saved sp with true return address. */
-	"	movl %eax, 15*4(%esp)\n"
 	RESTORE_REGS_STRING
+	"	addl $4, %esp\n"
 	"	popfl\n"
 #endif
 	"	ret\n"
@@ -1073,8 +1076,10 @@  NOKPROBE_SYMBOL(kretprobe_trampoline);
 /*
  * Called from kretprobe_trampoline
  */
-__used __visible void *trampoline_handler(struct pt_regs *regs)
+__used __visible void trampoline_handler(struct pt_regs *regs)
 {
+	unsigned long *frame_pointer;
+
 	/* fixup registers */
 	regs->cs = __KERNEL_CS;
 #ifdef CONFIG_X86_32
@@ -1082,8 +1087,16 @@  __used __visible void *trampoline_handler(struct pt_regs *regs)
 #endif
 	regs->ip = (unsigned long)&kretprobe_trampoline;
 	regs->orig_ax = ~0UL;
+	regs->sp += sizeof(long);
+	frame_pointer = ((unsigned long *)&regs->sp) + 1;
 
-	return (void *)kretprobe_trampoline_handler(regs, &regs->sp);
+	/* Replace fake return address with real one. */
+	*frame_pointer = kretprobe_trampoline_handler(regs, frame_pointer);
+	/*
+	 * Move flags to sp so that kretprobe_trapmoline can return
+	 * right after popf.
+	 */
+	regs->sp = regs->flags;
 }
 NOKPROBE_SYMBOL(trampoline_handler);