From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v15 04/10] arm64: Kprobes with single stepping support
Date: Wed, 20 Jul 2016 17:09:28 +0100 [thread overview]
Message-ID: <578FA238.3050206@arm.com> (raw)
In-Reply-To: <1467995754-32508-5-git-send-email-dave.long@linaro.org>
On 08/07/16 17:35, David Long wrote:
> From: Sandeepa Prabhu <sandeepa.s.prabhu@gmail•com>
>
> Add support for basic kernel probes(kprobes) and jump probes
> (jprobes) for ARM64.
>
> Kprobes utilizes software breakpoint and single step debug
> exceptions supported on ARM v8.
>
> A software breakpoint is placed at the probe address to trap the
> kernel execution into the kprobe handler.
>
> ARM v8 supports enabling single stepping before the break exception
> return (ERET), with next PC in exception return address (ELR_EL1). The
> kprobe handler prepares an executable memory slot for out-of-line
> execution with a copy of the original instruction being probed, and
> enables single stepping. The PC is set to the out-of-line slot address
> before the ERET. With this scheme, the instruction is executed with the
> exact same register context except for the PC (and DAIF) registers.
>
> Debug mask (PSTATE.D) is enabled only when single stepping a recursive
> kprobe, e.g.: during kprobes reenter so that probed instruction can be
> single stepped within the kprobe handler -exception- context.
> The recursion depth of kprobe is always 2, i.e. upon probe re-entry,
> any further re-entry is prevented by not calling handlers and the case
> counted as a missed kprobe).
>
> Single stepping from the x-o-l slot has a drawback for PC-relative accesses
> like branching and symbolic literals access as the offset from the new PC
> (slot address) may not be ensured to fit in the immediate value of
> the opcode. Such instructions need simulation, so reject
> probing them.
>
> Instructions generating exceptions or cpu mode change are rejected
> for probing.
>
> Exclusive load/store instructions are rejected too. Additionally, the
> code is checked to see if it is inside an exclusive load/store sequence
> (code from Pratyush).
>
> System instructions are mostly enabled for stepping, except MSR/MRS
> accesses to "DAIF" flags in PSTATE, which are not safe for
> probing.
>
> This also changes arch/arm64/include/asm/ptrace.h to use
> include/asm-generic/ptrace.h.
>
> Thanks to Steve Capper and Pratyush Anand for several suggested
> Changes.
>
> Signed-off-by: Sandeepa Prabhu <sandeepa.s.prabhu@gmail•com>
> Signed-off-by: David A. Long <dave.long@linaro•org>
> Signed-off-by: Pratyush Anand <panand@redhat•com>
> Acked-by: Masami Hiramatsu <mhiramat@kernel•org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/debug-monitors.h | 5 +
> arch/arm64/include/asm/insn.h | 2 +
> arch/arm64/include/asm/kprobes.h | 60 ++++
> arch/arm64/include/asm/probes.h | 34 +++
> arch/arm64/include/asm/ptrace.h | 14 +-
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/debug-monitors.c | 16 +-
> arch/arm64/kernel/probes/Makefile | 1 +
> arch/arm64/kernel/probes/decode-insn.c | 143 +++++++++
> arch/arm64/kernel/probes/decode-insn.h | 34 +++
> arch/arm64/kernel/probes/kprobes.c | 525 ++++++++++++++++++++++++++++++++
> arch/arm64/kernel/vmlinux.lds.S | 1 +
> arch/arm64/mm/fault.c | 26 ++
> 14 files changed, 859 insertions(+), 5 deletions(-)
> create mode 100644 arch/arm64/include/asm/kprobes.h
> create mode 100644 arch/arm64/include/asm/probes.h
> create mode 100644 arch/arm64/kernel/probes/Makefile
> create mode 100644 arch/arm64/kernel/probes/decode-insn.c
> create mode 100644 arch/arm64/kernel/probes/decode-insn.h
> create mode 100644 arch/arm64/kernel/probes/kprobes.c
>
[...]
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> new file mode 100644
> index 0000000..79c9511
> --- /dev/null
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -0,0 +1,60 @@
> +/*
> + * arch/arm64/include/asm/kprobes.h
> + *
> + * Copyright (C) 2013 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +
> +#ifndef _ARM_KPROBES_H
> +#define _ARM_KPROBES_H
> +
> +#include <linux/types.h>
> +#include <linux/ptrace.h>
> +#include <linux/percpu.h>
> +
> +#define __ARCH_WANT_KPROBES_INSN_SLOT
> +#define MAX_INSN_SIZE 1
> +#define MAX_STACK_SIZE 128
Where is that value coming from? Because even on my 6502, I have a 256
byte stack.
> +
> +#define flush_insn_slot(p) do { } while (0)
> +#define kretprobe_blacklist_size 0
> +
> +#include <asm/probes.h>
> +
> +struct prev_kprobe {
> + struct kprobe *kp;
> + unsigned int status;
> +};
> +
> +/* Single step context for kprobe */
> +struct kprobe_step_ctx {
> + unsigned long ss_pending;
> + unsigned long match_addr;
> +};
> +
> +/* per-cpu kprobe control block */
> +struct kprobe_ctlblk {
> + unsigned int kprobe_status;
> + unsigned long saved_irqflag;
> + struct prev_kprobe prev_kprobe;
> + struct kprobe_step_ctx ss_ctx;
> + struct pt_regs jprobe_saved_regs;
> + char jprobes_stack[MAX_STACK_SIZE];
Yeah, right. Let's keep this array in mind for a second.
> +};
> +
> +void arch_remove_kprobe(struct kprobe *);
> +int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr);
> +int kprobe_exceptions_notify(struct notifier_block *self,
> + unsigned long val, void *data);
> +int kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr);
> +int kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr);
> +
> +#endif /* _ARM_KPROBES_H */
> diff --git a/arch/arm64/include/asm/probes.h b/arch/arm64/include/asm/probes.h
> new file mode 100644
> index 0000000..1e8a21a
[...]
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> new file mode 100644
> index 0000000..4496801
> --- /dev/null
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -0,0 +1,525 @@
> +/*
> + * arch/arm64/kernel/probes/kprobes.c
> + *
> + * Kprobes support for ARM64
> + *
> + * Copyright (C) 2013 Linaro Limited.
> + * Author: Sandeepa Prabhu <sandeepa.prabhu@linaro•org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + */
> +#include <linux/kernel.h>
> +#include <linux/kprobes.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/stop_machine.h>
> +#include <linux/stringify.h>
> +#include <asm/traps.h>
> +#include <asm/ptrace.h>
> +#include <asm/cacheflush.h>
> +#include <asm/debug-monitors.h>
> +#include <asm/system_misc.h>
> +#include <asm/insn.h>
> +#include <asm/uaccess.h>
> +#include <asm/irq.h>
> +
> +#include "decode-insn.h"
> +
> +#define MIN_STACK_SIZE(addr) (on_irq_stack(addr, raw_smp_processor_id()) ? \
> + min((unsigned long)IRQ_STACK_SIZE, \
> + IRQ_STACK_PTR(raw_smp_processor_id()) - (addr)) : \
> + min((unsigned long)MAX_STACK_SIZE, \
> + (unsigned long)current_thread_info() + THREAD_START_SP - (addr)))
This macro makes me want to throw things at people, because there is no
way it can be reasonable parsed. So I've converted it to:
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 823cf92..5ee9c54 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -34,11 +34,23 @@
#include "decode-insn.h"
-#define MIN_STACK_SIZE(addr) (on_irq_stack(addr, raw_smp_processor_id()) ? \
- min((unsigned long)IRQ_STACK_SIZE, \
- IRQ_STACK_PTR(raw_smp_processor_id()) - (addr)) : \
- min((unsigned long)MAX_STACK_SIZE, \
- (unsigned long)current_thread_info() + THREAD_START_SP - (addr)))
+static unsigned long min_stack_size(unsigned long addr)
+{
+ unsigned long max_size;
+ unsigned long size;
+
+ if (on_irq_stack(addr, raw_smp_processor_id())) {
+ max_size = IRQ_STACK_SIZE;
+ size = IRQ_STACK_PTR(raw_smp_processor_id()) - addr;
+ } else {
+ max_size = MAX_STACK_SIZE;
+ size = (unsigned long)current_thread_info() + THREAD_START_SP - addr;
+ }
+
+ return min(size, max_size);
+}
+
+#define MIN_STACK_SIZE(addr) min_stack_size(addr)
void jprobe_return_break(void);
And then you can instrument it. If you add a simple printk to dump how
much you're going to copy, you get:
root at 10:/# nc -l -p 8080
size = 1248
size = 1248
Bad mode in Synchronous Abort handler detected on CPU0, code 0x86000006 -- IABT (current EL)
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc7-next-20160719-00068-g80315b6-dirty #6265
Hardware name: linux,dummy-virt (DT)
task: ffff000009020280 task.stack: ffff000009010000
PC is at 0x4000
LR is at enqueue_task_fair+0x8d8/0x1568
pc : [<0000000000004000>] lr : [<ffff000008101c78>] pstate: 200001c5
sp : ffff8000fffad7d0
Yes, 1248 bytes. How is that supposed to work?
So I've rewritten it like this:
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 823cf92..194a679 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -34,11 +34,20 @@
#include "decode-insn.h"
-#define MIN_STACK_SIZE(addr) (on_irq_stack(addr, raw_smp_processor_id()) ? \
- min((unsigned long)IRQ_STACK_SIZE, \
- IRQ_STACK_PTR(raw_smp_processor_id()) - (addr)) : \
- min((unsigned long)MAX_STACK_SIZE, \
- (unsigned long)current_thread_info() + THREAD_START_SP - (addr)))
+static inline unsigned long min_stack_size(unsigned long addr)
+{
+ unsigned long size;
+ struct kprobe_ctlblk *ctl;
+
+ if (on_irq_stack(addr, raw_smp_processor_id()))
+ size = IRQ_STACK_PTR(raw_smp_processor_id()) - addr;
+ else
+ size = (unsigned long)current_thread_info() + THREAD_START_SP - addr;
+
+ return min(size, sizeof(ctl->jprobes_stack));
+}
+
+#define MIN_STACK_SIZE(addr) min_stack_size(addr)
void jprobe_return_break(void);
I'm not sure if these 128 bytes are the right size for this thing,
but at least it won't blindly take the kernel down.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2016-07-20 16:09 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-08 16:35 [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support David Long
2016-07-08 16:35 ` [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature David Long
2016-07-15 10:57 ` Catalin Marinas
2016-07-15 14:51 ` David Long
2016-07-15 15:13 ` Catalin Marinas
2016-07-15 17:51 ` David Long
2016-07-19 14:17 ` Catalin Marinas
2016-07-08 16:35 ` [PATCH v15 02/10] arm64: Add more test functions to insn.c David Long
2016-07-08 16:35 ` [PATCH v15 03/10] arm64: add conditional instruction simulation support David Long
2016-07-08 16:35 ` [PATCH v15 04/10] arm64: Kprobes with single stepping support David Long
2016-07-20 9:36 ` Marc Zyngier
2016-07-20 11:16 ` Catalin Marinas
2016-07-20 19:08 ` David Long
2016-07-21 8:44 ` Marc Zyngier
2016-07-20 15:49 ` Catalin Marinas
2016-07-21 14:50 ` David Long
2016-07-20 16:09 ` Marc Zyngier [this message]
2016-07-20 16:28 ` Catalin Marinas
2016-07-20 16:31 ` Marc Zyngier
2016-07-20 16:46 ` Marc Zyngier
2016-07-20 17:04 ` Catalin Marinas
2016-07-21 16:33 ` David Long
2016-07-21 17:16 ` Catalin Marinas
2016-07-21 17:23 ` Marc Zyngier
2016-07-21 18:33 ` David Long
2016-07-22 10:16 ` Catalin Marinas
2016-07-22 15:51 ` David Long
2016-07-25 17:13 ` Catalin Marinas
2016-07-25 22:27 ` David Long
2016-07-27 11:50 ` Daniel Thompson
2016-07-27 22:13 ` David Long
2016-07-28 14:40 ` Catalin Marinas
2016-07-29 9:01 ` Daniel Thompson
2016-08-04 4:47 ` David Long
2016-08-08 11:13 ` Daniel Thompson
2016-08-08 14:29 ` David Long
2016-08-08 22:49 ` Masami Hiramatsu
2016-08-09 17:23 ` Catalin Marinas
2016-08-10 20:41 ` David Long
2016-08-08 22:19 ` Masami Hiramatsu
2016-07-26 9:50 ` Daniel Thompson
2016-07-26 16:55 ` Catalin Marinas
2016-07-27 10:01 ` Dave Martin
2016-07-26 17:54 ` Mark Rutland
2016-07-27 11:19 ` Daniel Thompson
2016-07-27 11:38 ` Dave Martin
2016-07-27 11:42 ` Daniel Thompson
2016-07-27 13:38 ` Mark Rutland
2016-07-08 16:35 ` [PATCH v15 05/10] arm64: Blacklist non-kprobe-able symbol David Long
2016-07-08 16:35 ` [PATCH v15 06/10] arm64: Treat all entry code as non-kprobe-able David Long
2016-07-15 16:47 ` Catalin Marinas
2016-07-19 0:53 ` David Long
2016-07-08 16:35 ` [PATCH v15 07/10] arm64: kprobes instruction simulation support David Long
2016-07-10 22:51 ` Paul Gortmaker
2016-07-08 16:35 ` [PATCH v15 08/10] arm64: Add trampoline code for kretprobes David Long
2016-07-19 13:46 ` Catalin Marinas
2016-07-20 18:28 ` David Long
2016-07-08 16:35 ` [PATCH v15 09/10] arm64: Add kernel return probes support (kretprobes) David Long
2016-07-08 16:35 ` [PATCH v15 10/10] kprobes: Add arm64 case in kprobe example module David Long
2016-07-14 16:22 ` [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Catalin Marinas
2016-07-14 17:09 ` William Cohen
2016-07-15 7:50 ` Catalin Marinas
2016-07-15 8:01 ` Marc Zyngier
2016-07-15 8:59 ` Alex Bennée
2016-07-15 9:04 ` Marc Zyngier
2016-07-15 9:53 ` Marc Zyngier
2016-07-14 17:56 ` David Long
2016-07-19 13:57 ` Catalin Marinas
2016-07-19 14:01 ` David Long
2016-07-19 18:27 ` Catalin Marinas
2016-07-19 19:38 ` David Long
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=578FA238.3050206@arm.com \
--to=marc.zyngier@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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