All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>, X86 ML <x86@kernel.org>,
	Daniel Xu <dxu@dxuuu.xyz>,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	kuba@kernel.org, mingo@redhat.com, ast@kernel.org,
	tglx@linutronix.de, kernel-team@fb.com, yhs@fb.com,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-ia64@vger.kernel.org
Subject: Re: [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in __kretprobe_trampoline_handler
Date: Sun, 21 Mar 2021 21:54:36 +0900	[thread overview]
Message-ID: <20210321215436.2e0447bf4fc43471bcecc243@kernel.org> (raw)
In-Reply-To: <161615659174.306069.12736134222759644948.stgit@devnote2>

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

WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>, X86 ML <x86@kernel.org>,
	Daniel Xu <dxu@dxuuu.xyz>,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	kuba@kernel.org, mingo@redhat.com, ast@kernel.org,
	tglx@linutronix.de, kernel-team@fb.com, yhs@fb.com,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-ia64@vger.kernel.org
Subject: Re: [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in __kretprobe_trampoline_handler
Date: Sun, 21 Mar 2021 12:54:36 +0000	[thread overview]
Message-ID: <20210321215436.2e0447bf4fc43471bcecc243@kernel.org> (raw)
In-Reply-To: <161615659174.306069.12736134222759644948.stgit@devnote2>

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

  reply	other threads:[~2021-03-21 12:55 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-19 12:21 [PATCH -tip v3 00/11] kprobes: Fix stacktrace with kretprobes on x86 Masami Hiramatsu
2021-03-19 12:21 ` Masami Hiramatsu
2021-03-19 12:21 ` [PATCH -tip v3 01/11] ia64: kprobes: Fix to pass correct trampoline address to the handler Masami Hiramatsu
2021-03-19 12:21   ` Masami Hiramatsu
2021-03-19 12:22 ` [PATCH -tip v3 02/11] kprobes: treewide: Replace arch_deref_entry_point() with dereference_function_descriptor() Masami Hiramatsu
2021-03-19 12:22   ` [PATCH -tip v3 02/11] kprobes: treewide: Replace arch_deref_entry_point() with dereference_function_ Masami Hiramatsu
2021-03-19 12:22 ` [PATCH -tip v3 03/11] kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() Masami Hiramatsu
2021-03-19 12:22   ` [PATCH -tip v3 03/11] kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler Masami Hiramatsu
2021-03-19 12:22 ` [PATCH -tip v3 04/11] kprobes: Add kretprobe_find_ret_addr() for searching return address Masami Hiramatsu
2021-03-19 12:22   ` Masami Hiramatsu
2021-03-19 12:22 ` [PATCH -tip v3 05/11] x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code Masami Hiramatsu
2021-03-19 12:22   ` Masami Hiramatsu
2021-03-20 12:16   ` Masami Hiramatsu
2021-03-20 12:16     ` Masami Hiramatsu
2021-03-20 13:05     ` Masami Hiramatsu
2021-03-20 13:05       ` Masami Hiramatsu
2021-03-21 17:52       ` Josh Poimboeuf
2021-03-21 17:52         ` Josh Poimboeuf
2021-03-22  0:08         ` Masami Hiramatsu
2021-03-22  0:08           ` Masami Hiramatsu
2021-03-21 17:50     ` Josh Poimboeuf
2021-03-21 17:50       ` Josh Poimboeuf
2021-03-19 12:22 ` [PATCH -tip v3 06/11] ARC: Add instruction_pointer_set() API Masami Hiramatsu
2021-03-19 12:22   ` Masami Hiramatsu
2021-03-19 12:23 ` [PATCH -tip v3 07/11] ia64: " Masami Hiramatsu
2021-03-19 12:23   ` Masami Hiramatsu
2021-03-20  1:04   ` Masami Hiramatsu
2021-03-20  1:04     ` Masami Hiramatsu
2021-03-19 12:23 ` [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in __kretprobe_trampoline_handler Masami Hiramatsu
2021-03-19 12:23   ` Masami Hiramatsu
2021-03-21 12:54   ` Masami Hiramatsu [this message]
2021-03-21 12:54     ` Masami Hiramatsu
2021-03-19 12:23 ` [PATCH -tip v3 09/11] x86/kprobes: Push a fake return address at kretprobe_trampoline Masami Hiramatsu
2021-03-19 12:23   ` Masami Hiramatsu
2021-03-19 12:23 ` [PATCH -tip v3 10/11] x86/unwind: Recover kretprobe trampoline entry Masami Hiramatsu
2021-03-19 12:23   ` Masami Hiramatsu
2021-03-19 12:23 ` [PATCH -tip v3 11/11] tracing: Show kretprobe unknown indicator only for kretprobe_trampoline Masami Hiramatsu
2021-03-19 12:23   ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210321215436.2e0447bf4fc43471bcecc243@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dxu@dxuuu.xyz \
    --cc=jpoimboe@redhat.com \
    --cc=kernel-team@fb.com \
    --cc=kuba@kernel.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.