Re: [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in __kretprobe_trampoline_handler

[Masami Hiramatsu]

On Fri, 19 Mar 2021 21:23:12 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:

> To simplify the stacktrace with pt_regs from kretprobe handler,
> set the correct return address to the instruction pointer in
> the pt_regs before calling kretprobe handlers.
> 

Oops, now I also find this breaks kretprobe for arm. It seems
not enough registers stores. I need to fix the arm kprobe code
too.

In arch/arm/include/uapi/asm/ptrace.h,

---
#define ARM_pc          uregs[15]
---

And in arch/arm/probes/kprobes/core.c,

---
/*
 * When a retprobed function returns, trampoline_handler() is called,
 * calling the kretprobe's handler. We construct a struct pt_regs to
 * give a view of registers r0-r11 to the user return-handler.  This is
 * not a complete pt_regs structure, but that should be plenty sufficient
 * for kretprobe handlers which should normally be interested in r0 only
 * anyway.
 */
void __naked __kprobes kretprobe_trampoline(void)
{
        __asm__ __volatile__ (
                "stmdb  sp!, {r0 - r11}         \n\t"
---

So, changing regs->ARM_pc will break a stack entry. I need to expand
it to r15.

Thanks,

> Suggested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> ---
>  Changes in v3:
>   - Cast the correct_ret_addr to unsigned long.
> ---
>  kernel/kprobes.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index cf19edc038e4..4ce3e6f5d28d 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1914,6 +1914,9 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,
>  		BUG_ON(1);
>  	}
>  
> +	/* Set the instruction pointer to the correct address */
> +	instruction_pointer_set(regs, (unsigned long)correct_ret_addr);
> +
>  	/* Run them. */
>  	first = current->kretprobe_instances.first;
>  	while (first) {
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>