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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210321215436.2e0447bf4fc43471bcecc243@kernel.org>
Date:   Sun, 21 Mar 2021 21:54:36 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>, X86 ML <x86@...nel.org>,
        Daniel Xu <dxu@...uu.xyz>, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org, kuba@...nel.org, mingo@...hat.com,
        ast@...nel.org, tglx@...utronix.de, kernel-team@...com, yhs@...com,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        linux-ia64@...r.kernel.org
Subject: Re: [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in
 __kretprobe_trampoline_handler

On Fri, 19 Mar 2021 21:23:12 +0900
Masami Hiramatsu <mhiramat@...nel.org> 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@...hat.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> ---
>  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@...nel.org>