From: Masami Hiramatsu <mhiramat@kernel.org>
To: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Jonathan Corbet <corbet@lwn.net>,
Catalin Marinas <catalin.marinas@arm.com>,
"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
Ingo Molnar <mingo@redhat.com>,
Steven Rostedt <rostedt@goodmis.org>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
Will Deacon <will@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v7 3/3] arm64: implement KPROBES_ON_FTRACE
Date: Thu, 26 Dec 2019 11:57:07 +0900 [thread overview]
Message-ID: <20191226115707.902545688aa90b34e2e550b3@kernel.org> (raw)
In-Reply-To: <20191225173001.6c0e3fb2@xhacker.debian>
Hi Jisheng,
On Wed, 25 Dec 2019 09:44:21 +0000
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
> KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
> eliminates the need for a trap, as well as the need to emulate or
> single-step instructions.
>
> Tested on berlin arm64 platform.
>
> ~ # mount -t debugfs debugfs /sys/kernel/debug/
> ~ # cd /sys/kernel/debug/
> /sys/kernel/debug # echo 'p _do_fork' > tracing/kprobe_events
>
> before the patch:
>
> /sys/kernel/debug # cat kprobes/list
> ffffff801009fe28 k _do_fork+0x0 [DISABLED]
>
> after the patch:
>
> /sys/kernel/debug # cat kprobes/list
> ffffff801009ff54 k _do_fork+0x0 [DISABLED][FTRACE]
What happens if user puts a probe on _do_fork+4?
Is that return -EILSEQ correctly?
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/ftrace.h | 1 +
> arch/arm64/kernel/probes/Makefile | 1 +
> arch/arm64/kernel/probes/ftrace.c | 78 +++++++++++++++++++
> 5 files changed, 82 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kernel/probes/ftrace.c
>
> diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> index 4fae0464ddff..f9dd9dd91e0c 100644
> --- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> +++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> @@ -9,7 +9,7 @@
> | alpha: | TODO |
> | arc: | TODO |
> | arm: | TODO |
> - | arm64: | TODO |
> + | arm64: | ok |
> | c6x: | TODO |
> | csky: | TODO |
> | h8300: | TODO |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b1b4476ddb83..92b9882889ac 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -166,6 +166,7 @@ config ARM64
> select HAVE_STACKPROTECTOR
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_KPROBES
> + select HAVE_KPROBES_ON_FTRACE
> select HAVE_KRETPROBES
> select HAVE_GENERIC_VDSO
> select IOMMU_DMA if IOMMU_SUPPORT
> diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> index 91fa4baa1a93..875aeb839654 100644
> --- a/arch/arm64/include/asm/ftrace.h
> +++ b/arch/arm64/include/asm/ftrace.h
> @@ -20,6 +20,7 @@
>
> /* The BL at the callsite's adjusted rec->ip */
> #define MCOUNT_INSN_SIZE AARCH64_INSN_SIZE
> +#define FTRACE_IP_EXTENSION MCOUNT_INSN_SIZE
>
> #define FTRACE_PLT_IDX 0
> #define FTRACE_REGS_PLT_IDX 1
> diff --git a/arch/arm64/kernel/probes/Makefile b/arch/arm64/kernel/probes/Makefile
> index 8e4be92e25b1..4020cfc66564 100644
> --- a/arch/arm64/kernel/probes/Makefile
> +++ b/arch/arm64/kernel/probes/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o \
> simulate-insn.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o \
> simulate-insn.o
> +obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> diff --git a/arch/arm64/kernel/probes/ftrace.c b/arch/arm64/kernel/probes/ftrace.c
> new file mode 100644
> index 000000000000..0643aa2dacdb
> --- /dev/null
> +++ b/arch/arm64/kernel/probes/ftrace.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Dynamic Ftrace based Kprobes Optimization
> + *
> + * Copyright (C) Hitachi Ltd., 2012
> + * Copyright (C) 2019 Jisheng Zhang <jszhang@kernel.org>
> + * Synaptics Incorporated
> + */
> +
> +#include <linux/kprobes.h>
> +
> +/*
> + * In arm64 FTRACE_WITH_REGS implementation, we patch two nop instructions:
> + * the lr saver and bl ftrace-entry. Both these instructions are claimed
> + * by ftrace and we should allow probing on either instruction.
No, the 2nd bl ftrace-entry must not be probed.
The pair of lr-saver and bl ftrace-entry is tightly coupled. You can not
decouple it.
> + */
> +int arch_check_ftrace_location(struct kprobe *p)
> +{
> + if (ftrace_location((unsigned long)p->addr))
> + p->flags |= KPROBE_FLAG_FTRACE;
> + return 0;
> +}
Thus, this must return -EILSEQ if user puts a probe on the bl.
> +
> +/* Ftrace callback handler for kprobes -- called under preepmt disabed */
> +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> + bool lr_saver = false;
> + struct kprobe *p;
> + struct kprobe_ctlblk *kcb;
> +
> + /* Preempt is disabled by ftrace */
> + p = get_kprobe((kprobe_opcode_t *)ip);
> + if (!p) {
> + p = get_kprobe((kprobe_opcode_t *)(ip - MCOUNT_INSN_SIZE));
> + if (unlikely(!p) || kprobe_disabled(p))
> + return;
> + lr_saver = true;
Then, this can be removed.
> + }
> +
> + kcb = get_kprobe_ctlblk();
> + if (kprobe_running()) {
> + kprobes_inc_nmissed_count(p);
> + } else {
> + unsigned long orig_ip = instruction_pointer(regs);
> +
> + if (lr_saver)
> + ip -= MCOUNT_INSN_SIZE;
Ditto.
Thank you,
> + instruction_pointer_set(regs, ip);
> + __this_cpu_write(current_kprobe, p);
> + kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> + if (!p->pre_handler || !p->pre_handler(p, regs)) {
> + /*
> + * Emulate singlestep (and also recover regs->pc)
> + * as if there is a nop
> + */
> + instruction_pointer_set(regs,
> + (unsigned long)p->addr + MCOUNT_INSN_SIZE);
> + if (unlikely(p->post_handler)) {
> + kcb->kprobe_status = KPROBE_HIT_SSDONE;
> + p->post_handler(p, regs, 0);
> + }
> + instruction_pointer_set(regs, orig_ip);
> + }
> + /*
> + * If pre_handler returns !0, it changes regs->pc. We have to
> + * skip emulating post_handler.
> + */
> + __this_cpu_write(current_kprobe, NULL);
> + }
> +}
> +NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> +
> +int arch_prepare_kprobe_ftrace(struct kprobe *p)
> +{
> + p->ainsn.api.insn = NULL;
> + return 0;
> +}
> --
> 2.24.1
>
--
Masami Hiramatsu <mhiramat@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-12-26 2:57 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-25 9:40 [PATCH v7 0/3] arm64: implement KPROBES_ON_FTRACE Jisheng Zhang
2019-12-25 9:42 ` [PATCH v7 1/3] kprobes/ftrace: Use ftrace_location() when [dis]arming probes Jisheng Zhang
2019-12-25 9:46 ` Jisheng Zhang
2019-12-25 9:42 ` [PATCH v7 2/3] ftrace: introduce FTRACE_IP_EXTENSION Jisheng Zhang
2019-12-26 2:45 ` Masami Hiramatsu
2020-01-08 0:05 ` Steven Rostedt
2019-12-25 9:44 ` [PATCH v7 3/3] arm64: implement KPROBES_ON_FTRACE Jisheng Zhang
2019-12-26 2:57 ` Masami Hiramatsu [this message]
2019-12-26 3:18 ` Jisheng Zhang
2019-12-26 4:25 ` Jisheng Zhang
2019-12-26 9:26 ` Masami Hiramatsu
2020-07-21 13:24 ` Masami Hiramatsu
2020-07-24 7:06 ` Jisheng Zhang
2020-07-24 16:54 ` Masami Hiramatsu
2020-02-28 15:31 ` Mark Rutland
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=20191226115707.902545688aa90b34e2e550b3@kernel.org \
--to=mhiramat@kernel.org \
--cc=Jisheng.Zhang@synaptics.com \
--cc=anil.s.keshavamurthy@intel.com \
--cc=catalin.marinas@arm.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=naveen.n.rao@linux.ibm.com \
--cc=rostedt@goodmis.org \
--cc=will@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).