On Fri, May 10, 2019 at 01:58:31PM +0900, Masami Hiramatsu wrote:
On Thu, 9 May 2019 19:14:16 +0200 Peter Zijlstra peterz@infradead.org wrote:
--- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -731,29 +731,8 @@ asm( ".global kretprobe_trampoline\n" ".type kretprobe_trampoline, @function\n" "kretprobe_trampoline:\n"
Here, we need a gap for storing ret-ip, because kretprobe_trampoline is the address which is returned from the target function. We have no "ret-ip" here at this point. So something like
- "push $0\n" /* This is a gap, will be filled with real return address*/
The trampoline already provides a gap, trampoline_handler() will need to use int3_emulate_push() if it wants to inject something on the return stack.
I guess you mean the int3 case. This trampoline is used as a return destination.
When the target function is called, kretprobe interrupts the first instruction, and replace the return address with this trampoline. When a "ret" instruction is done, it returns to this trampoline. Thus the stack frame start with previous context here. As you described above,
I would prefer to change that to inject an extra return address, instead of replacing it. With the new exception stuff we can actually do that.
So on entry we then go from:
<previous context> RET-IP
to
<previous context> RET-IP return-trampoline
So when the function returns, it falls into the trampoline instead.
* On entry the stack looks like:
*
* 2*4(%esp) <previous context>
* 1*4(%esp) RET-IP
* 0*4(%esp) func
From this trampoline call, the stack looks like:
* 1*4(%esp) <previous context> * 0*4(%esp) func
So we need one more push.
And then the stack looks just right at this point.
- "push trampoline_handler\n"
- "jmp call_to_exception_trampoline\n" ".size kretprobe_trampoline, .-kretprobe_trampoline\n"
); NOKPROBE_SYMBOL(kretprobe_trampoline);