Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1439656 > unrolled thread

[PATCH v15 00/10] arm64: Add kernel probes (kprobes) support

Back to article view | Back to linux.kernel

Contents

  [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support David Long <dave.long@linaro.org> - 2016-07-08 18:40 +0200
    [PATCH v15 05/10] arm64: Blacklist non-kprobe-able symbol David Long <dave.long@linaro.org> - 2016-07-08 18:40 +0200
    [PATCH v15 08/10] arm64: Add trampoline code for kretprobes David Long <dave.long@linaro.org> - 2016-07-08 18:40 +0200
      Re: [PATCH v15 08/10] arm64: Add trampoline code for kretprobes Catalin Marinas <catalin.marinas@arm.com> - 2016-07-19 15:50 +0200
        Re: [PATCH v15 08/10] arm64: Add trampoline code for kretprobes David Long <dave.long@linaro.org> - 2016-07-20 20:30 +0200
    [PATCH v15 09/10] arm64: Add kernel return probes support (kretprobes) David Long <dave.long@linaro.org> - 2016-07-08 18:40 +0200
    [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature David Long <dave.long@linaro.org> - 2016-07-08 18:40 +0200
      Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API  feature Catalin Marinas <catalin.marinas@arm.com> - 2016-07-15 13:00 +0200
        Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API  feature David Long <dave.long@linaro.org> - 2016-07-15 17:00 +0200
          Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API  feature Catalin Marinas <catalin.marinas@arm.com> - 2016-07-15 17:20 +0200
            Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API  feature David Long <dave.long@linaro.org> - 2016-07-15 20:00 +0200
              Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API  feature Catalin Marinas <catalin.marinas@arm.com> - 2016-07-19 16:20 +0200
    Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Catalin Marinas <catalin.marinas@arm.com> - 2016-07-14 18:30 +0200
      Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support William Cohen <wcohen@redhat.com> - 2016-07-14 19:10 +0200
        Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Catalin Marinas <catalin.marinas@arm.com> - 2016-07-15 10:00 +0200
          Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Marc Zyngier <marc.zyngier@arm.com> - 2016-07-15 10:10 +0200
            Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Alex Bennée <alex.bennee@linaro.org> - 2016-07-15 11:00 +0200
              Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Marc Zyngier <marc.zyngier@arm.com> - 2016-07-15 11:10 +0200
              Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Marc Zyngier <marc.zyngier@arm.com> - 2016-07-15 12:00 +0200
      Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support David Long <dave.long@linaro.org> - 2016-07-14 20:00 +0200
      Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Catalin Marinas <catalin.marinas@arm.com> - 2016-07-19 16:00 +0200
        Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support David Long <dave.long@linaro.org> - 2016-07-19 16:10 +0200
    Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support Catalin Marinas <catalin.marinas@arm.com> - 2016-07-19 20:30 +0200
      Re: [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support David Long <dave.long@linaro.org> - 2016-07-19 21:40 +0200

Page 1 of 2  [1] 2  Next page →

#1439656 — [PATCH v15 00/10] arm64: Add kernel probes (kprobes) support

From: "David A. Long" <dave.long@linaro.org>

This patchset is heavily based on Sandeepa Prabhu's ARM v8 kprobes patches,
first seen in October 2013. This version attempts to address concerns
raised by reviewers and also fixes problems discovered during testing.

This patchset adds support for kernel probes(kprobes), jump probes(jprobes)
and return probes(kretprobes) support for ARM64.

The kprobes mechanism makes use of software breakpoint and single stepping
support available in the ARM v8 kernel.

Changes since v2 include:

1) Removal of NOP padding in kprobe XOL slots. Slots are now exactly one
instruction long.
2) Disabling of interrupts during execution in single-step mode.
3) Fixing of numerous problems in instruction simulation code (mostly
thanks to Will Cohen).
4) Support for the HAVE_REGS_AND_STACK_ACCESS_API feature is added, to
allow access to kprobes through debugfs.
5) kprobes is *not* enabled in defconfig.
6) Numerous complaints from checkpatch have been cleaned up, although a
couple remain as removing the function pointer typedefs results in ugly
code.

Changes since v3 include:

1) Remove table-driven instruction parsing and replace with an if statement
calling out to old and new instruction test functions in insn.c.
2) I removed the addition of orig_x0 to ptrace.h.
3) Reorder the patches.
4) Replace the previous interrupt disabling (from Will Cohen) with
an improved solution (from Steve Capper).

Changes since v4 include:

1) Added insn.c functions to detect exception instructions and DAIF
   read/write instructions, and use them to reject probing same.
2) Changed adr detect function to also recognize adrp. Reject both.
3) Added missing __kprobes for some new functions.
4) Added call to kprobes_fault_handler from mm do_page_fault.
5) Reject all non-simulated branch/ret instructions, not just those
   that use an immediate offset.
6) Moved software breakpoint definitions into debug-monitors.h.
7) Removed "!XIP_KERNEL" from Kconfig.
8) changed kprobes_condition_check_t and kprobes_prepare_t to probes_*,
   for future sharing with uprobes.
9) Removed bogus call to kprobes_restore_local_irqflag() from 
   trampoline_probe_handler().

Changes since v5 include:

1) Replaced installation of breakpoint hook with direct call from the
handlers in debug-monitors.c, as requested.
2) Reject probing of instructions that read the interrupt mask, in
addition to instructions that set it.
3) Cleaned up comments describing usage of Debug Mask.
4) Added KPROBE_REENTER case in reenter_kprobe.
5) Corrected the ifdef'd definitions for notify_page_fault() to be
consistent when KPROBES is not configed.
6) Changed "cpsr" to "pstate" for HAVE_REGS_AND_STACK_ACCESS_API feature.
7) Added back in missing new files in previous patch.
8) Changed two instances of pr_warning() to pr_warn().

Note that there seems to be at least a potential issue with kprobes
on multiple (possibly all) platforms having to do with use of kfree
inside of the kretprobes trampoline handler.  This has manifested
occasionally in systemtap testing on arm64.  There does not appear to
be an simple solution to the problem.

Changes since v6 include:

1) New trampoline code from Will Cohen fixes the occasional failure seen
when processing kretprobes by replacing the software breakpoint with
assembly code to implement the return to the original execution stream.
2) Changed ip0, ip1, fp, and lr to plain numbered registers for purposes
of recognizing them as an ascii string in the stack/reg access code.
3) Removed orig_x0.
4) Moved ARM_x* defines from arch/arm64/include/uapi/asm/ptrace.h to
arch/arm64/kernel/ptrace.c.

Changes since v7 include:

1) Move trampoline entry/return code into separate ".S" file instead
of making it a macro in a header file.
2) Add missing register name definitions in asm-offsets.c and use them
in place of hard-coded integer offsets in the trampoline code.
3) Correct the values used to decode MSR immediate instructions, in insn.h.
4) Remove the currently unused simulate_none() function.

Changes since v8 include:

1) Replaced use of REG_OFFSET_NAME with GPR_OFFSET_NAME for numbered
registers.
2) Added an alias for "lr" in the register name lookup table, which perf
tools need to be able to recognize.
3) Changed the code for checking instruction types for probeability and
steppability as per review feedback.
4) Fixed the size of cache being flushed when filling single-step slot.
5) Fixed big-endian issues.
6) Blacklisted copy_to/from_user to avoid aborts while single-stepping.
7) Record conditional instructions that fail the conditional test just
like any other probed (non-conditional) instruction.
8) Removed use of magic number for detecting jprobe return and just
check the breakpoint address instead.
9) Got rid of the unnecessary arch/arm64/kprobes.h.
10) The PSTATE and SP are now properly saved in the kretprobe trampoline
code.
11) This patch no longer depends on the "Consolidate redundant
register/stack access code" patch set.
12) Remove call to fixup_exception from kprobe_fault_handler.

Changes since v9 include:

1) Remove arch/arm/opcodes.c from the arm64 build and move the renamed
arm64_check_condition() function to armv8_deprecated.c. Remove the
asmlinkage.
2) Various other type and style changes suggested by Marc Zyngier.
3) Put back the call to fixup_exception from kprobe_fault_handler.
It proved to be necessary for correct operation.

Changes since v10 include:

1) Rename arm64_check_condition() to arm32_check_condition().
2) Remove redundant define of ARM_OPCODE_CONDITION_UNCOND.
3) Use a accessor functions to read and write registers by number
in the simulation code, to avoid accidentally overriding parts of
the pt_regs structure (e.g.: when the reg is xzr).
4) Remove unused register offset defines.
5) Replace instance of "(void *) 0" with NULL.
6) Rewrite the kretprobe trampoline code using arch/arm64/kvm/hyp/entry.S
as an example. Construct a more complete saved PSTATE in this code.

Changes since v11 include:

1) Add check for address within irq stack, in regs_within_kernel_stack()
2) Replaced inappropriate use of user_pt_regs with pt_regs.
3) Added comments to opcode_condition_checks table explaining equivalence
of "nv" and "al" condition codes.
4) Cleaned up some subtle problems in the instruction simulation code.
5) Readability improvements in kprobes_trampoline.S.
6) Additional blacklisting for entry code, exception handling code, and
select debug functions.
7) Check address to be probed for proper alignment.
8) Add rodata section to areas where kprobes may not be placed.

Changes since v12 include:

1) Changed regs_get_register() to expicitly reference pt_regs structure
fields instead of just using an address offset.
2) Reject probing of eret.
3) Correctly handle addresses on the interrupt stack
4) Add kprobe_ctlblk argument to static irqflag handling functions to avoid
doing extra calls to get_kprobe_ctlblk().
5) Removed a couple of logically redundant assignments to kprobe_status.
6) Added calls to pause_graph_tracing/unpause_graph_tracing to avoid
disaster when kprobe'ing and tracing at the same time.
7) Added idmap and hypervisor text sections to blacklisted regions
8) Numerous additional comments, formatting changes, and rearranging
of if-else statements.

Changes since v13 include:

1) Fixed regs_get_register() from previous version to correctly calculate
the offset of registers in struct pt_regs.
2) I removed the removal of the typecast inside the instruction_pointer()
define in ptrace.h, and added a define for instruction_pointer_set(). This
was necessary to correct warnings that were being emitted when compiling
kgdb code.
3) Removed a redundant/bogus "NOKPROBE_SYMBOL(do_debug_exception)"
statement.
4) Fixed aarch64_insn_extract_system_reg() from previous version to use the
correct name "aarch64_insn_extract_system_reg()".
5) Changed opcode_condition_checks[] to aarch32_opcode_cond_checks[] and
arm32_check_condition() to aarch32_check_condition().
6) I switched the order of the main kprobes patch and the symbol function
blacklisting patch back to the order they were done in the earlier patches.
7) I got rid of struct kprobe_pc_restore and now just use a non-zero saved
PC as the flag to restore the PC.
8) I changed the names of some of the arm64 kprobes source files and moved
then into their own "kprobes" subdirectory under arch/arm64/kernel.
9) I moved the INSN_GOOD_NO_SLOT enum value to the later commit that makes
use of it.
10) I added kernel_disable_single_stap() and spsr_set_debug_flag() calls in
kprobe_fault_handler() for the KPROBE_REENTER case.
11) I brought trampoline_probe_handler() up to date with x86 sources to
pick up a fix from Syuhei (commit 737480a0d525).
12) I changed samples/kprobes/kprobe_example.c modifications to more
closely match what is currently done for other architectures.

Changes since v14:

1) Change the name of arch/arm64/kernel/kprobes to
arch/arm64/kernel/probes/ and fix the name in the Makefile and comments.
2) Include include/linux/asm-generic/ptrace.h in
arch/arm64/include/asm/ptrace.h, add the required definitions in the latter
to make this work, and add a couple typecasts in
arch/arm64/kernel/probes/kprobes.c to accomodate this.
3) added Ack's to commits.

David A. Long (3):
  arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
  arm64: Add more test functions to insn.c
  arm64: add conditional instruction simulation support

Pratyush Anand (2):
  arm64: Blacklist non-kprobe-able symbol
  arm64: Treat all entry code as non-kprobe-able

Sandeepa Prabhu (4):
  arm64: Kprobes with single stepping support
  arm64: kprobes instruction simulation support
  arm64: Add kernel return probes support (kretprobes)
  kprobes: Add arm64 case in kprobe example module

William Cohen (1):
  arm64: Add trampoline code for kretprobes

 arch/arm64/Kconfig                            |   3 +
 arch/arm64/include/asm/debug-monitors.h       |   5 +
 arch/arm64/include/asm/insn.h                 |  41 ++
 arch/arm64/include/asm/kprobes.h              |  62 +++
 arch/arm64/include/asm/probes.h               |  35 ++
 arch/arm64/include/asm/ptrace.h               |  66 ++-
 arch/arm64/kernel/Makefile                    |   5 +-
 arch/arm64/kernel/arm64ksyms.c                |   2 +
 arch/arm64/kernel/armv8_deprecated.c          |  19 +-
 arch/arm64/kernel/asm-offsets.c               |  11 +
 arch/arm64/kernel/debug-monitors.c            |  33 +-
 arch/arm64/kernel/entry.S                     |   3 +
 arch/arm64/kernel/hw_breakpoint.c             |   8 +
 arch/arm64/kernel/insn.c                      | 133 +++++
 arch/arm64/kernel/kgdb.c                      |   4 +
 arch/arm64/kernel/probes/Makefile             |   3 +
 arch/arm64/kernel/probes/decode-insn.c        | 174 +++++++
 arch/arm64/kernel/probes/decode-insn.h        |  35 ++
 arch/arm64/kernel/probes/kprobes.c            | 675 ++++++++++++++++++++++++++
 arch/arm64/kernel/probes/kprobes_trampoline.S |  85 ++++
 arch/arm64/kernel/probes/simulate-insn.c      | 218 +++++++++
 arch/arm64/kernel/probes/simulate-insn.h      |  28 ++
 arch/arm64/kernel/ptrace.c                    | 118 +++++
 arch/arm64/kernel/vmlinux.lds.S               |   2 +
 arch/arm64/mm/fault.c                         |  26 +
 samples/kprobes/kprobe_example.c              |   9 +
 26 files changed, 1794 insertions(+), 9 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
 create mode 100644 arch/arm64/kernel/probes/kprobes_trampoline.S
 create mode 100644 arch/arm64/kernel/probes/simulate-insn.c
 create mode 100644 arch/arm64/kernel/probes/simulate-insn.h

-- 
2.5.0

[toc] | [next] | [standalone]

#1439658 — [PATCH v15 05/10] arm64: Blacklist non-kprobe-able symbol

From: Pratyush Anand <panand@redhat.com>

Add all function symbols which are called from do_debug_exception under
NOKPROBE_SYMBOL, as they can not kprobed.

Signed-off-by: Pratyush Anand <panand@redhat.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/kernel/arm64ksyms.c     |  2 ++
 arch/arm64/kernel/debug-monitors.c | 17 +++++++++++++++++
 arch/arm64/kernel/hw_breakpoint.c  |  8 ++++++++
 arch/arm64/kernel/kgdb.c           |  4 ++++
 4 files changed, 31 insertions(+)

diff --git a/arch/arm64/kernel/arm64ksyms.c b/arch/arm64/kernel/arm64ksyms.c
index 678f30b0..b96ff1a 100644
--- a/arch/arm64/kernel/arm64ksyms.c
+++ b/arch/arm64/kernel/arm64ksyms.c
@@ -27,6 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/io.h>
 #include <linux/arm-smccc.h>
+#include <linux/kprobes.h>
 
 #include <asm/checksum.h>
 
@@ -68,6 +69,7 @@ EXPORT_SYMBOL(test_and_change_bit);
 
 #ifdef CONFIG_FUNCTION_TRACER
 EXPORT_SYMBOL(_mcount);
+NOKPROBE_SYMBOL(_mcount);
 #endif
 
 	/* arm-smccc */
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 395de61..2fbc1b9 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -49,6 +49,7 @@ static void mdscr_write(u32 mdscr)
 	asm volatile("msr mdscr_el1, %0" :: "r" (mdscr));
 	local_dbg_restore(flags);
 }
+NOKPROBE_SYMBOL(mdscr_write);
 
 static u32 mdscr_read(void)
 {
@@ -56,6 +57,7 @@ static u32 mdscr_read(void)
 	asm volatile("mrs %0, mdscr_el1" : "=r" (mdscr));
 	return mdscr;
 }
+NOKPROBE_SYMBOL(mdscr_read);
 
 /*
  * Allow root to disable self-hosted debug from userspace.
@@ -104,6 +106,7 @@ void enable_debug_monitors(enum dbg_active_el el)
 		mdscr_write(mdscr);
 	}
 }
+NOKPROBE_SYMBOL(enable_debug_monitors);
 
 void disable_debug_monitors(enum dbg_active_el el)
 {
@@ -124,6 +127,7 @@ void disable_debug_monitors(enum dbg_active_el el)
 		mdscr_write(mdscr);
 	}
 }
+NOKPROBE_SYMBOL(disable_debug_monitors);
 
 /*
  * OS lock clearing.
@@ -174,6 +178,7 @@ static void set_regs_spsr_ss(struct pt_regs *regs)
 	spsr |= DBG_SPSR_SS;
 	regs->pstate = spsr;
 }
+NOKPROBE_SYMBOL(set_regs_spsr_ss);
 
 static void clear_regs_spsr_ss(struct pt_regs *regs)
 {
@@ -183,6 +188,7 @@ static void clear_regs_spsr_ss(struct pt_regs *regs)
 	spsr &= ~DBG_SPSR_SS;
 	regs->pstate = spsr;
 }
+NOKPROBE_SYMBOL(clear_regs_spsr_ss);
 
 /* EL1 Single Step Handler hooks */
 static LIST_HEAD(step_hook);
@@ -226,6 +232,7 @@ static int call_step_hook(struct pt_regs *regs, unsigned int esr)
 
 	return retval;
 }
+NOKPROBE_SYMBOL(call_step_hook);
 
 static void send_user_sigtrap(int si_code)
 {
@@ -284,6 +291,7 @@ static int single_step_handler(unsigned long addr, unsigned int esr,
 
 	return 0;
 }
+NOKPROBE_SYMBOL(single_step_handler);
 
 /*
  * Breakpoint handler is re-entrant as another breakpoint can
@@ -321,6 +329,7 @@ static int call_break_hook(struct pt_regs *regs, unsigned int esr)
 
 	return fn ? fn(regs, esr) : DBG_HOOK_ERROR;
 }
+NOKPROBE_SYMBOL(call_break_hook);
 
 static int brk_handler(unsigned long addr, unsigned int esr,
 		       struct pt_regs *regs)
@@ -341,6 +350,7 @@ static int brk_handler(unsigned long addr, unsigned int esr,
 
 	return 0;
 }
+NOKPROBE_SYMBOL(brk_handler);
 
 int aarch32_break_handler(struct pt_regs *regs)
 {
@@ -377,6 +387,7 @@ int aarch32_break_handler(struct pt_regs *regs)
 	send_user_sigtrap(TRAP_BRKPT);
 	return 0;
 }
+NOKPROBE_SYMBOL(aarch32_break_handler);
 
 static int __init debug_traps_init(void)
 {
@@ -398,6 +409,7 @@ void user_rewind_single_step(struct task_struct *task)
 	if (test_ti_thread_flag(task_thread_info(task), TIF_SINGLESTEP))
 		set_regs_spsr_ss(task_pt_regs(task));
 }
+NOKPROBE_SYMBOL(user_rewind_single_step);
 
 void user_fastforward_single_step(struct task_struct *task)
 {
@@ -413,6 +425,7 @@ void kernel_enable_single_step(struct pt_regs *regs)
 	mdscr_write(mdscr_read() | DBG_MDSCR_SS);
 	enable_debug_monitors(DBG_ACTIVE_EL1);
 }
+NOKPROBE_SYMBOL(kernel_enable_single_step);
 
 void kernel_disable_single_step(void)
 {
@@ -420,12 +433,14 @@ void kernel_disable_single_step(void)
 	mdscr_write(mdscr_read() & ~DBG_MDSCR_SS);
 	disable_debug_monitors(DBG_ACTIVE_EL1);
 }
+NOKPROBE_SYMBOL(kernel_disable_single_step);
 
 int kernel_active_single_step(void)
 {
 	WARN_ON(!irqs_disabled());
 	return mdscr_read() & DBG_MDSCR_SS;
 }
+NOKPROBE_SYMBOL(kernel_active_single_step);
 
 /* ptrace API */
 void user_enable_single_step(struct task_struct *task)
@@ -433,8 +448,10 @@ void user_enable_single_step(struct task_struct *task)
 	set_ti_thread_flag(task_thread_info(task), TIF_SINGLESTEP);
 	set_regs_spsr_ss(task_pt_regs(task));
 }
+NOKPROBE_SYMBOL(user_enable_single_step);
 
 void user_disable_single_step(struct task_struct *task)
 {
 	clear_ti_thread_flag(task_thread_info(task), TIF_SINGLESTEP);
 }
+NOKPROBE_SYMBOL(user_disable_single_step);
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index ce21aa8..26a6bf7 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -24,6 +24,7 @@
 #include <linux/cpu_pm.h>
 #include <linux/errno.h>
 #include <linux/hw_breakpoint.h>
+#include <linux/kprobes.h>
 #include <linux/perf_event.h>
 #include <linux/ptrace.h>
 #include <linux/smp.h>
@@ -127,6 +128,7 @@ static u64 read_wb_reg(int reg, int n)
 
 	return val;
 }
+NOKPROBE_SYMBOL(read_wb_reg);
 
 static void write_wb_reg(int reg, int n, u64 val)
 {
@@ -140,6 +142,7 @@ static void write_wb_reg(int reg, int n, u64 val)
 	}
 	isb();
 }
+NOKPROBE_SYMBOL(write_wb_reg);
 
 /*
  * Convert a breakpoint privilege level to the corresponding exception
@@ -157,6 +160,7 @@ static enum dbg_active_el debug_exception_level(int privilege)
 		return -EINVAL;
 	}
 }
+NOKPROBE_SYMBOL(debug_exception_level);
 
 enum hw_breakpoint_ops {
 	HW_BREAKPOINT_INSTALL,
@@ -575,6 +579,7 @@ static void toggle_bp_registers(int reg, enum dbg_active_el el, int enable)
 		write_wb_reg(reg, i, ctrl);
 	}
 }
+NOKPROBE_SYMBOL(toggle_bp_registers);
 
 /*
  * Debug exception handlers.
@@ -654,6 +659,7 @@ unlock:
 
 	return 0;
 }
+NOKPROBE_SYMBOL(breakpoint_handler);
 
 static int watchpoint_handler(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)
@@ -756,6 +762,7 @@ unlock:
 
 	return 0;
 }
+NOKPROBE_SYMBOL(watchpoint_handler);
 
 /*
  * Handle single-step exception.
@@ -813,6 +820,7 @@ int reinstall_suspended_bps(struct pt_regs *regs)
 
 	return !handled_exception;
 }
+NOKPROBE_SYMBOL(reinstall_suspended_bps);
 
 /*
  * Context-switcher for restoring suspended breakpoints.
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index b5f063e..8c57f64 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -22,6 +22,7 @@
 #include <linux/irq.h>
 #include <linux/kdebug.h>
 #include <linux/kgdb.h>
+#include <linux/kprobes.h>
 #include <asm/traps.h>
 
 struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
@@ -230,6 +231,7 @@ static int kgdb_brk_fn(struct pt_regs *regs, unsigned int esr)
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
 	return 0;
 }
+NOKPROBE_SYMBOL(kgdb_brk_fn)
 
 static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
@@ -238,12 +240,14 @@ static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int esr)
 
 	return 0;
 }
+NOKPROBE_SYMBOL(kgdb_compiled_brk_fn);
 
 static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
 	return 0;
 }
+NOKPROBE_SYMBOL(kgdb_step_brk_fn);
 
 static struct break_hook kgdb_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1439662 — [PATCH v15 08/10] arm64: Add trampoline code for kretprobes

From: William Cohen <wcohen@redhat.com>

The trampoline code is used by kretprobes to capture a return from a probed
function.  This is done by saving the registers, calling the handler, and
restoring the registers. The code then returns to the original saved caller
return address. It is necessary to do this directly instead of using a
software breakpoint because the code used in processing that breakpoint
could itself be kprobe'd and cause a problematic reentry into the debug
exception handler.

Signed-off-by: William Cohen <wcohen@redhat.com>
Signed-off-by: David A. Long <dave.long@linaro.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/include/asm/kprobes.h              |  2 +
 arch/arm64/kernel/asm-offsets.c               | 11 ++++
 arch/arm64/kernel/probes/Makefile             |  1 +
 arch/arm64/kernel/probes/kprobes.c            |  5 ++
 arch/arm64/kernel/probes/kprobes_trampoline.S | 85 +++++++++++++++++++++++++++
 5 files changed, 104 insertions(+)
 create mode 100644 arch/arm64/kernel/probes/kprobes_trampoline.S

diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
index 79c9511..61b4915 100644
--- a/arch/arm64/include/asm/kprobes.h
+++ b/arch/arm64/include/asm/kprobes.h
@@ -56,5 +56,7 @@ 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);
+void kretprobe_trampoline(void);
+void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
 
 #endif /* _ARM_KPROBES_H */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index f8e5d47..03dfa27 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -51,6 +51,17 @@ int main(void)
   DEFINE(S_X5,			offsetof(struct pt_regs, regs[5]));
   DEFINE(S_X6,			offsetof(struct pt_regs, regs[6]));
   DEFINE(S_X7,			offsetof(struct pt_regs, regs[7]));
+  DEFINE(S_X8,			offsetof(struct pt_regs, regs[8]));
+  DEFINE(S_X10,			offsetof(struct pt_regs, regs[10]));
+  DEFINE(S_X12,			offsetof(struct pt_regs, regs[12]));
+  DEFINE(S_X14,			offsetof(struct pt_regs, regs[14]));
+  DEFINE(S_X16,			offsetof(struct pt_regs, regs[16]));
+  DEFINE(S_X18,			offsetof(struct pt_regs, regs[18]));
+  DEFINE(S_X20,			offsetof(struct pt_regs, regs[20]));
+  DEFINE(S_X22,			offsetof(struct pt_regs, regs[22]));
+  DEFINE(S_X24,			offsetof(struct pt_regs, regs[24]));
+  DEFINE(S_X26,			offsetof(struct pt_regs, regs[26]));
+  DEFINE(S_X28,			offsetof(struct pt_regs, regs[28]));
   DEFINE(S_LR,			offsetof(struct pt_regs, regs[30]));
   DEFINE(S_SP,			offsetof(struct pt_regs, sp));
 #ifdef CONFIG_COMPAT
diff --git a/arch/arm64/kernel/probes/Makefile b/arch/arm64/kernel/probes/Makefile
index e184d00..ce06312 100644
--- a/arch/arm64/kernel/probes/Makefile
+++ b/arch/arm64/kernel/probes/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_KPROBES)		+= kprobes.o decode-insn.o	\
+				   kprobes_trampoline.o		\
 				   simulate-insn.o
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 63eb0a1..be1f074 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -576,6 +576,11 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
 	return false;
 }
 
+void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
+{
+	return NULL;
+}
+
 int __init arch_init_kprobes(void)
 {
 	return 0;
diff --git a/arch/arm64/kernel/probes/kprobes_trampoline.S b/arch/arm64/kernel/probes/kprobes_trampoline.S
new file mode 100644
index 0000000..ba37d85
--- /dev/null
+++ b/arch/arm64/kernel/probes/kprobes_trampoline.S
@@ -0,0 +1,85 @@
+/*
+ * trampoline entry and return code for kretprobes.
+ */
+
+#include <linux/linkage.h>
+#include <asm/asm-offsets.h>
+#include <asm/assembler.h>
+
+	.text
+
+.macro save_all_base_regs
+	stp x0, x1, [sp, #S_X0]
+	stp x2, x3, [sp, #S_X2]
+	stp x4, x5, [sp, #S_X4]
+	stp x6, x7, [sp, #S_X6]
+	stp x8, x9, [sp, #S_X8]
+	stp x10, x11, [sp, #S_X10]
+	stp x12, x13, [sp, #S_X12]
+	stp x14, x15, [sp, #S_X14]
+	stp x16, x17, [sp, #S_X16]
+	stp x18, x19, [sp, #S_X18]
+	stp x20, x21, [sp, #S_X20]
+	stp x22, x23, [sp, #S_X22]
+	stp x24, x25, [sp, #S_X24]
+	stp x26, x27, [sp, #S_X26]
+	stp x28, x29, [sp, #S_X28]
+	add x0, sp, #S_FRAME_SIZE
+	stp lr, x0, [sp, #S_LR]
+/*
+ * Construct a useful saved PSTATE
+ */
+	mrs x0, nzcv
+	and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
+	mrs x1, daif
+	and x1, x1, #(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
+	orr x0, x0, x1
+	mrs x1, CurrentEL
+	and x1, x1, #(3 << 2)
+	orr x0, x1, x0
+	mrs x1, SPSel
+	and x1, x1, #1
+	orr x0, x1, x0
+	str x0, [sp, #S_PSTATE]
+.endm
+
+.macro restore_all_base_regs
+	ldr x0, [sp, #S_PSTATE]
+	and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
+	msr nzcv, x0
+	ldp x0, x1, [sp, #S_X0]
+	ldp x2, x3, [sp, #S_X2]
+	ldp x4, x5, [sp, #S_X4]
+	ldp x6, x7, [sp, #S_X6]
+	ldp x8, x9, [sp, #S_X8]
+	ldp x10, x11, [sp, #S_X10]
+	ldp x12, x13, [sp, #S_X12]
+	ldp x14, x15, [sp, #S_X14]
+	ldp x16, x17, [sp, #S_X16]
+	ldp x18, x19, [sp, #S_X18]
+	ldp x20, x21, [sp, #S_X20]
+	ldp x22, x23, [sp, #S_X22]
+	ldp x24, x25, [sp, #S_X24]
+	ldp x26, x27, [sp, #S_X26]
+	ldp x28, x29, [sp, #S_X28]
+.endm
+
+ENTRY(kretprobe_trampoline)
+
+	sub sp, sp, #S_FRAME_SIZE
+
+	save_all_base_regs
+
+	mov x0, sp
+	bl trampoline_probe_handler
+	/* Replace trampoline address in lr with actual
+	   orig_ret_addr return address. */
+	mov lr, x0
+
+	restore_all_base_regs
+
+	add sp, sp, #S_FRAME_SIZE
+
+	ret
+
+ENDPROC(kretprobe_trampoline)
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1446490 — Re: [PATCH v15 08/10] arm64: Add trampoline code for kretprobes

On Fri, Jul 08, 2016 at 12:35:52PM -0400, David Long wrote:
> --- /dev/null
> +++ b/arch/arm64/kernel/probes/kprobes_trampoline.S
> @@ -0,0 +1,85 @@
> +/*
> + * trampoline entry and return code for kretprobes.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/assembler.h>
> +
> +	.text
> +
> +.macro save_all_base_regs
> +	stp x0, x1, [sp, #S_X0]
> +	stp x2, x3, [sp, #S_X2]
> +	stp x4, x5, [sp, #S_X4]
> +	stp x6, x7, [sp, #S_X6]
> +	stp x8, x9, [sp, #S_X8]
> +	stp x10, x11, [sp, #S_X10]
> +	stp x12, x13, [sp, #S_X12]
> +	stp x14, x15, [sp, #S_X14]
> +	stp x16, x17, [sp, #S_X16]
> +	stp x18, x19, [sp, #S_X18]
> +	stp x20, x21, [sp, #S_X20]
> +	stp x22, x23, [sp, #S_X22]
> +	stp x24, x25, [sp, #S_X24]
> +	stp x26, x27, [sp, #S_X26]
> +	stp x28, x29, [sp, #S_X28]
> +	add x0, sp, #S_FRAME_SIZE
> +	stp lr, x0, [sp, #S_LR]
> +/*
> + * Construct a useful saved PSTATE
> + */
> +	mrs x0, nzcv
> +	and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
> +	mrs x1, daif
> +	and x1, x1, #(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)

I don't think you need the masking here, the mrs should return the
corresponding 4 bits.

> +	orr x0, x0, x1
> +	mrs x1, CurrentEL
> +	and x1, x1, #(3 << 2)
> +	orr x0, x1, x0
> +	mrs x1, SPSel
> +	and x1, x1, #1

Same here.

> +	orr x0, x1, x0
> +	str x0, [sp, #S_PSTATE]
> +.endm

How is this pstate used, other than the restoring of the condition flag
in the restore_all_base_regs macro? Does a kretprobes handler need
access to them?

Anyway, it's worth doing an stp xzr, x0, [sp, S_PC] so that we
initialise the pc in pt_regs.

> +
> +.macro restore_all_base_regs
> +	ldr x0, [sp, #S_PSTATE]
> +	and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
> +	msr nzcv, x0
> +	ldp x0, x1, [sp, #S_X0]
> +	ldp x2, x3, [sp, #S_X2]
> +	ldp x4, x5, [sp, #S_X4]
> +	ldp x6, x7, [sp, #S_X6]
> +	ldp x8, x9, [sp, #S_X8]
> +	ldp x10, x11, [sp, #S_X10]
> +	ldp x12, x13, [sp, #S_X12]
> +	ldp x14, x15, [sp, #S_X14]
> +	ldp x16, x17, [sp, #S_X16]
> +	ldp x18, x19, [sp, #S_X18]
> +	ldp x20, x21, [sp, #S_X20]
> +	ldp x22, x23, [sp, #S_X22]
> +	ldp x24, x25, [sp, #S_X24]
> +	ldp x26, x27, [sp, #S_X26]
> +	ldp x28, x29, [sp, #S_X28]
> +.endm
> +
> +ENTRY(kretprobe_trampoline)
> +
> +	sub sp, sp, #S_FRAME_SIZE
> +
> +	save_all_base_regs
> +
> +	mov x0, sp
> +	bl trampoline_probe_handler
> +	/* Replace trampoline address in lr with actual
> +	   orig_ret_addr return address. */
> +	mov lr, x0
> +
> +	restore_all_base_regs
> +
> +	add sp, sp, #S_FRAME_SIZE
> +
> +	ret
> +
> +ENDPROC(kretprobe_trampoline)

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1447389 — Re: [PATCH v15 08/10] arm64: Add trampoline code for kretprobes

On 07/19/2016 09:46 AM, Catalin Marinas wrote:
> On Fri, Jul 08, 2016 at 12:35:52PM -0400, David Long wrote:
>> --- /dev/null
>> +++ b/arch/arm64/kernel/probes/kprobes_trampoline.S
>> @@ -0,0 +1,85 @@
>> +/*
>> + * trampoline entry and return code for kretprobes.
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/assembler.h>
>> +
>> +	.text
>> +
>> +.macro save_all_base_regs
>> +	stp x0, x1, [sp, #S_X0]
>> +	stp x2, x3, [sp, #S_X2]
>> +	stp x4, x5, [sp, #S_X4]
>> +	stp x6, x7, [sp, #S_X6]
>> +	stp x8, x9, [sp, #S_X8]
>> +	stp x10, x11, [sp, #S_X10]
>> +	stp x12, x13, [sp, #S_X12]
>> +	stp x14, x15, [sp, #S_X14]
>> +	stp x16, x17, [sp, #S_X16]
>> +	stp x18, x19, [sp, #S_X18]
>> +	stp x20, x21, [sp, #S_X20]
>> +	stp x22, x23, [sp, #S_X22]
>> +	stp x24, x25, [sp, #S_X24]
>> +	stp x26, x27, [sp, #S_X26]
>> +	stp x28, x29, [sp, #S_X28]
>> +	add x0, sp, #S_FRAME_SIZE
>> +	stp lr, x0, [sp, #S_LR]
>> +/*
>> + * Construct a useful saved PSTATE
>> + */
>> +	mrs x0, nzcv
>> +	and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
>> +	mrs x1, daif
>> +	and x1, x1, #(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
>
> I don't think you need the masking here, the mrs should return the
> corresponding 4 bits.
>

OK. I see you've done that.

>> +	orr x0, x0, x1
>> +	mrs x1, CurrentEL
>> +	and x1, x1, #(3 << 2)
>> +	orr x0, x1, x0
>> +	mrs x1, SPSel
>> +	and x1, x1, #1
>
> Same here.

OK. ^

>
>> +	orr x0, x1, x0
>> +	str x0, [sp, #S_PSTATE]
>> +.endm
>
> How is this pstate used, other than the restoring of the condition flag
> in the restore_all_base_regs macro? Does a kretprobes handler need
> access to them?
>

A kretprobes handler should probably be able to examine a reasonable 
pstate value, particularly in terms of DAIF. As I recall not having a 
valid DAIF was an issue at one time.

> Anyway, it's worth doing an stp xzr, x0, [sp, S_PC] so that we
> initialise the pc in pt_regs.
>

OK.  Looks like you've done this.

>> +
>> +.macro restore_all_base_regs
>> +	ldr x0, [sp, #S_PSTATE]
>> +	and x0, x0, #(PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT)
>> +	msr nzcv, x0
>> +	ldp x0, x1, [sp, #S_X0]
>> +	ldp x2, x3, [sp, #S_X2]
>> +	ldp x4, x5, [sp, #S_X4]
>> +	ldp x6, x7, [sp, #S_X6]
>> +	ldp x8, x9, [sp, #S_X8]
>> +	ldp x10, x11, [sp, #S_X10]
>> +	ldp x12, x13, [sp, #S_X12]
>> +	ldp x14, x15, [sp, #S_X14]
>> +	ldp x16, x17, [sp, #S_X16]
>> +	ldp x18, x19, [sp, #S_X18]
>> +	ldp x20, x21, [sp, #S_X20]
>> +	ldp x22, x23, [sp, #S_X22]
>> +	ldp x24, x25, [sp, #S_X24]
>> +	ldp x26, x27, [sp, #S_X26]
>> +	ldp x28, x29, [sp, #S_X28]
>> +.endm
>> +
>> +ENTRY(kretprobe_trampoline)
>> +
>> +	sub sp, sp, #S_FRAME_SIZE
>> +
>> +	save_all_base_regs
>> +
>> +	mov x0, sp
>> +	bl trampoline_probe_handler
>> +	/* Replace trampoline address in lr with actual
>> +	   orig_ret_addr return address. */
>> +	mov lr, x0
>> +
>> +	restore_all_base_regs
>> +
>> +	add sp, sp, #S_FRAME_SIZE
>> +
>> +	ret
>> +
>> +ENDPROC(kretprobe_trampoline)
>

Thanks,
-dl

[toc] | [prev] | [next] | [standalone]

#1439663 — [PATCH v15 09/10] arm64: Add kernel return probes support (kretprobes)

From: Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>

The pre-handler of this special 'trampoline' kprobe executes the return
probe handler functions and restores original return address in ELR_EL1.
This way the saved pt_regs still hold the original register context to be
carried back to the probed kernel function.

Signed-off-by: Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>
Signed-off-by: David A. Long <dave.long@linaro.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/Kconfig                 |  1 +
 arch/arm64/kernel/probes/kprobes.c | 90 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1f7d644..6af0e2e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -89,6 +89,7 @@ config ARM64
 	select HAVE_RCU_TABLE_FREE
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_KPROBES
+	select HAVE_KRETPROBES if HAVE_KPROBES
 	select IOMMU_DMA if IOMMU_SUPPORT
 	select IRQ_DOMAIN
 	select IRQ_FORCED_THREADING
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index be1f074..9c70e88 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -578,7 +578,95 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
 
 void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
 {
-	return NULL;
+	struct kretprobe_instance *ri = NULL;
+	struct hlist_head *head, empty_rp;
+	struct hlist_node *tmp;
+	unsigned long flags, orig_ret_address = 0;
+	unsigned long trampoline_address =
+		(unsigned long)&kretprobe_trampoline;
+	kprobe_opcode_t *correct_ret_addr = NULL;
+
+	INIT_HLIST_HEAD(&empty_rp);
+	kretprobe_hash_lock(current, &head, &flags);
+
+	/*
+	 * It is possible to have multiple instances associated with a given
+	 * task either because multiple functions in the call path have
+	 * return probes installed on them, and/or more than one
+	 * return probe was registered for a target function.
+	 *
+	 * We can handle this because:
+	 *     - instances are always pushed into the head of the list
+	 *     - when multiple return probes are registered for the same
+	 *	 function, the (chronologically) first instance's ret_addr
+	 *	 will be the real return address, and all the rest will
+	 *	 point to kretprobe_trampoline.
+	 */
+	hlist_for_each_entry_safe(ri, tmp, head, hlist) {
+		if (ri->task != current)
+			/* another task is sharing our hash bucket */
+			continue;
+
+		orig_ret_address = (unsigned long)ri->ret_addr;
+
+		if (orig_ret_address != trampoline_address)
+			/*
+			 * This is the real return address. Any other
+			 * instances associated with this task are for
+			 * other calls deeper on the call stack
+			 */
+			break;
+	}
+
+	kretprobe_assert(ri, orig_ret_address, trampoline_address);
+
+	correct_ret_addr = ri->ret_addr;
+	hlist_for_each_entry_safe(ri, tmp, head, hlist) {
+		if (ri->task != current)
+			/* another task is sharing our hash bucket */
+			continue;
+
+		orig_ret_address = (unsigned long)ri->ret_addr;
+		if (ri->rp && ri->rp->handler) {
+			__this_cpu_write(current_kprobe, &ri->rp->kp);
+			get_kprobe_ctlblk()->kprobe_status = KPROBE_HIT_ACTIVE;
+			ri->ret_addr = correct_ret_addr;
+			ri->rp->handler(ri, regs);
+			__this_cpu_write(current_kprobe, NULL);
+		}
+
+		recycle_rp_inst(ri, &empty_rp);
+
+		if (orig_ret_address != trampoline_address)
+			/*
+			 * This is the real return address. Any other
+			 * instances associated with this task are for
+			 * other calls deeper on the call stack
+			 */
+			break;
+	}
+
+	kretprobe_hash_unlock(current, &flags);
+
+	hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
+		hlist_del(&ri->hlist);
+		kfree(ri);
+	}
+	return (void *)orig_ret_address;
+}
+
+void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
+				      struct pt_regs *regs)
+{
+	ri->ret_addr = (kprobe_opcode_t *)regs->regs[30];
+
+	/* replace return addr (x30) with trampoline */
+	regs->regs[30] = (long)&kretprobe_trampoline;
+}
+
+int __kprobes arch_trampoline_kprobe(struct kprobe *p)
+{
+	return 0;
 }
 
 int __init arch_init_kprobes(void)
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1439665 — [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature

From: "David A. Long" <dave.long@linaro.org>

Add HAVE_REGS_AND_STACK_ACCESS_API feature for arm64, including supporting
functions and defines.

Signed-off-by: David A. Long <dave.long@linaro.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/Kconfig              |   1 +
 arch/arm64/include/asm/ptrace.h |  52 ++++++++++++++++++
 arch/arm64/kernel/ptrace.c      | 118 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5a0a691..fab133c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -85,6 +85,7 @@ config ARM64
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
+	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_RCU_TABLE_FREE
 	select HAVE_SYSCALL_TRACEPOINTS
 	select IOMMU_DMA if IOMMU_SUPPORT
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index a307eb6..6c0c7d3 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -74,6 +74,7 @@
 #define COMPAT_PT_DATA_ADDR		0x10004
 #define COMPAT_PT_TEXT_END_ADDR		0x10008
 #ifndef __ASSEMBLY__
+#include <linux/bug.h>
 
 /* sizeof(struct user) for AArch32 */
 #define COMPAT_USER_SZ	296
@@ -119,6 +120,8 @@ struct pt_regs {
 	u64 syscallno;
 };
 
+#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate)
+
 #define arch_has_single_step()	(1)
 
 #ifdef CONFIG_COMPAT
@@ -147,6 +150,55 @@ struct pt_regs {
 #define user_stack_pointer(regs) \
 	(!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp)
 
+extern int regs_query_register_offset(const char *name);
+extern const char *regs_query_register_name(unsigned int offset);
+extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr);
+extern unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
+					       unsigned int n);
+
+/**
+ * regs_get_register() - get register value from its offset
+ * @regs:	   pt_regs from which register value is gotten
+ * @offset:    offset of the register.
+ *
+ * regs_get_register returns the value of a register whose offset from @regs.
+ * The @offset is the offset of the register in struct pt_regs.
+ * If @offset is bigger than MAX_REG_OFFSET, this returns 0.
+ */
+static inline u64 regs_get_register(struct pt_regs *regs,
+					      unsigned int offset)
+{
+	u64 val = 0;
+
+	WARN_ON(offset & 7);
+
+	offset >>= 3;
+	switch (offset) {
+	case	0 ... 30:
+		val = regs->regs[offset];
+		break;
+	case offsetof(struct pt_regs, sp) >> 3:
+		val = regs->sp;
+		break;
+	case offsetof(struct pt_regs, pc) >> 3:
+		val = regs->pc;
+		break;
+	case offsetof(struct pt_regs, pstate) >> 3:
+		val = regs->pstate;
+		break;
+	default:
+		val = 0;
+	}
+
+	return val;
+}
+
+/* Valid only for Kernel mode traps. */
+static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
+{
+	return regs->sp;
+}
+
 static inline unsigned long regs_return_value(struct pt_regs *regs)
 {
 	return regs->regs[0];
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 3f6cd5c..2c88c33 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -48,6 +48,124 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>
 
+struct pt_regs_offset {
+	const char *name;
+	int offset;
+};
+
+#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs, r)}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+#define	GPR_OFFSET_NAME(r)	\
+	{.name = "x" #r, .offset = offsetof(struct pt_regs, regs[r])}
+
+static const struct pt_regs_offset regoffset_table[] = {
+	GPR_OFFSET_NAME(0),
+	GPR_OFFSET_NAME(1),
+	GPR_OFFSET_NAME(2),
+	GPR_OFFSET_NAME(3),
+	GPR_OFFSET_NAME(4),
+	GPR_OFFSET_NAME(5),
+	GPR_OFFSET_NAME(6),
+	GPR_OFFSET_NAME(7),
+	GPR_OFFSET_NAME(8),
+	GPR_OFFSET_NAME(9),
+	GPR_OFFSET_NAME(10),
+	GPR_OFFSET_NAME(11),
+	GPR_OFFSET_NAME(12),
+	GPR_OFFSET_NAME(13),
+	GPR_OFFSET_NAME(14),
+	GPR_OFFSET_NAME(15),
+	GPR_OFFSET_NAME(16),
+	GPR_OFFSET_NAME(17),
+	GPR_OFFSET_NAME(18),
+	GPR_OFFSET_NAME(19),
+	GPR_OFFSET_NAME(20),
+	GPR_OFFSET_NAME(21),
+	GPR_OFFSET_NAME(22),
+	GPR_OFFSET_NAME(23),
+	GPR_OFFSET_NAME(24),
+	GPR_OFFSET_NAME(25),
+	GPR_OFFSET_NAME(26),
+	GPR_OFFSET_NAME(27),
+	GPR_OFFSET_NAME(28),
+	GPR_OFFSET_NAME(29),
+	GPR_OFFSET_NAME(30),
+	{.name = "lr", .offset = offsetof(struct pt_regs, regs[30])},
+	REG_OFFSET_NAME(sp),
+	REG_OFFSET_NAME(pc),
+	REG_OFFSET_NAME(pstate),
+	REG_OFFSET_END,
+};
+
+/**
+ * regs_query_register_offset() - query register offset from its name
+ * @name:	the name of a register
+ *
+ * regs_query_register_offset() returns the offset of a register in struct
+ * pt_regs from its name. If the name is invalid, this returns -EINVAL;
+ */
+int regs_query_register_offset(const char *name)
+{
+	const struct pt_regs_offset *roff;
+
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (!strcmp(roff->name, name))
+			return roff->offset;
+	return -EINVAL;
+}
+
+/**
+ * regs_query_register_name() - query register name from its offset
+ * @offset:	the offset of a register in struct pt_regs.
+ *
+ * regs_query_register_name() returns the name of a register from its
+ * offset in struct pt_regs. If the @offset is invalid, this returns NULL;
+ */
+const char *regs_query_register_name(unsigned int offset)
+{
+	const struct pt_regs_offset *roff;
+
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (roff->offset == offset)
+			return roff->name;
+	return NULL;
+}
+
+/**
+ * regs_within_kernel_stack() - check the address in the stack
+ * @regs:      pt_regs which contains kernel stack pointer.
+ * @addr:      address which is checked.
+ *
+ * regs_within_kernel_stack() checks @addr is within the kernel stack page(s).
+ * If @addr is within the kernel stack, it returns true. If not, returns false.
+ */
+bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
+{
+	return ((addr & ~(THREAD_SIZE - 1))  ==
+		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1))) ||
+		on_irq_stack(addr, raw_smp_processor_id());
+}
+
+/**
+ * regs_get_kernel_stack_nth() - get Nth entry of the stack
+ * @regs:	pt_regs which contains kernel stack pointer.
+ * @n:		stack entry number.
+ *
+ * regs_get_kernel_stack_nth() returns @n th entry of the kernel stack which
+ * is specified by @regs. If the @n th entry is NOT in the kernel stack,
+ * this returns 0.
+ */
+unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+	unsigned long *addr = (unsigned long *)kernel_stack_pointer(regs);
+
+	addr += n;
+	if (regs_within_kernel_stack(regs, (unsigned long)addr))
+		return *addr;
+	else
+		return 0;
+}
+
 /*
  * TODO: does not yet catch signals sent when the child dies.
  * in exit.c or in signal.c.
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1444185 — Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature

On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote:
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -74,6 +74,7 @@
>  #define COMPAT_PT_DATA_ADDR		0x10004
>  #define COMPAT_PT_TEXT_END_ADDR		0x10008
>  #ifndef __ASSEMBLY__
> +#include <linux/bug.h>
>  
>  /* sizeof(struct user) for AArch32 */
>  #define COMPAT_USER_SZ	296
> @@ -119,6 +120,8 @@ struct pt_regs {
>  	u64 syscallno;
>  };
>  
> +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate)
> +
>  #define arch_has_single_step()	(1)
>  
>  #ifdef CONFIG_COMPAT
> @@ -147,6 +150,55 @@ struct pt_regs {
>  #define user_stack_pointer(regs) \
>  	(!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp)
>  
> +extern int regs_query_register_offset(const char *name);
> +extern const char *regs_query_register_name(unsigned int offset);

Is regs_query_register_offset() used anywhere? I grep'ed the kernel with
these patches applied but couldn't find any use.

> +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr);

This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we
make it static and remove the declaration?

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1444354 — Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature

On 07/15/2016 06:57 AM, Catalin Marinas wrote:
> On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote:
>> --- a/arch/arm64/include/asm/ptrace.h
>> +++ b/arch/arm64/include/asm/ptrace.h
>> @@ -74,6 +74,7 @@
>>   #define COMPAT_PT_DATA_ADDR		0x10004
>>   #define COMPAT_PT_TEXT_END_ADDR		0x10008
>>   #ifndef __ASSEMBLY__
>> +#include <linux/bug.h>
>>
>>   /* sizeof(struct user) for AArch32 */
>>   #define COMPAT_USER_SZ	296
>> @@ -119,6 +120,8 @@ struct pt_regs {
>>   	u64 syscallno;
>>   };
>>
>> +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate)
>> +
>>   #define arch_has_single_step()	(1)
>>
>>   #ifdef CONFIG_COMPAT
>> @@ -147,6 +150,55 @@ struct pt_regs {
>>   #define user_stack_pointer(regs) \
>>   	(!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp)
>>
>> +extern int regs_query_register_offset(const char *name);
>> +extern const char *regs_query_register_name(unsigned int offset);
>
> Is regs_query_register_offset() used anywhere? I grep'ed the kernel with
> these patches applied but couldn't find any use.
>

It's referenced in kernel/trace/trace_probe.c.

>> +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr);
>
> This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we
> make it static and remove the declaration?
>

OK.

Thanks,
-dl

[toc] | [prev] | [next] | [standalone]

#1444370 — Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature

On Fri, Jul 15, 2016 at 10:51:23AM -0400, David Long wrote:
> On 07/15/2016 06:57 AM, Catalin Marinas wrote:
> > On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote:
> > > --- a/arch/arm64/include/asm/ptrace.h
> > > +++ b/arch/arm64/include/asm/ptrace.h
> > > @@ -74,6 +74,7 @@
> > >   #define COMPAT_PT_DATA_ADDR		0x10004
> > >   #define COMPAT_PT_TEXT_END_ADDR		0x10008
> > >   #ifndef __ASSEMBLY__
> > > +#include <linux/bug.h>
> > > 
> > >   /* sizeof(struct user) for AArch32 */
> > >   #define COMPAT_USER_SZ	296
> > > @@ -119,6 +120,8 @@ struct pt_regs {
> > >   	u64 syscallno;
> > >   };
> > > 
> > > +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate)
> > > +
> > >   #define arch_has_single_step()	(1)
> > > 
> > >   #ifdef CONFIG_COMPAT
> > > @@ -147,6 +150,55 @@ struct pt_regs {
> > >   #define user_stack_pointer(regs) \
> > >   	(!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp)
> > > 
> > > +extern int regs_query_register_offset(const char *name);
> > > +extern const char *regs_query_register_name(unsigned int offset);
> > 
> > Is regs_query_register_offset() used anywhere? I grep'ed the kernel with
> > these patches applied but couldn't find any use.
> 
> It's referenced in kernel/trace/trace_probe.c.

I meant regs_query_register_name() (vim completion wrote the first one).

> > > +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr);
> > 
> > This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we
> > make it static and remove the declaration?
> 
> OK.

I can change it locally.

Are these going to be used in the future by uprobes?

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1444491 — Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature

On 07/15/2016 11:13 AM, Catalin Marinas wrote:
> On Fri, Jul 15, 2016 at 10:51:23AM -0400, David Long wrote:
>> On 07/15/2016 06:57 AM, Catalin Marinas wrote:
>>> On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote:
>>>> --- a/arch/arm64/include/asm/ptrace.h
>>>> +++ b/arch/arm64/include/asm/ptrace.h
>>>> @@ -74,6 +74,7 @@
>>>>    #define COMPAT_PT_DATA_ADDR		0x10004
>>>>    #define COMPAT_PT_TEXT_END_ADDR		0x10008
>>>>    #ifndef __ASSEMBLY__
>>>> +#include <linux/bug.h>
>>>>
>>>>    /* sizeof(struct user) for AArch32 */
>>>>    #define COMPAT_USER_SZ	296
>>>> @@ -119,6 +120,8 @@ struct pt_regs {
>>>>    	u64 syscallno;
>>>>    };
>>>>
>>>> +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate)
>>>> +
>>>>    #define arch_has_single_step()	(1)
>>>>
>>>>    #ifdef CONFIG_COMPAT
>>>> @@ -147,6 +150,55 @@ struct pt_regs {
>>>>    #define user_stack_pointer(regs) \
>>>>    	(!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp)
>>>>
>>>> +extern int regs_query_register_offset(const char *name);
>>>> +extern const char *regs_query_register_name(unsigned int offset);
>>>
>>> Is regs_query_register_offset() used anywhere? I grep'ed the kernel with
>>> these patches applied but couldnperf_regs.c't find any use.
>>
>> It's referenced in kernel/trace/trace_probe.c.
>
> I meant regs_query_register_name() (vim completion wrote the first one).
>

I had assumed it was used by kgdb to provide human-readable register 
names for debugger output, but apparently that is handled inside the 
kgdb.c stub for each architecture. I can only assume this is currently 
provided in all (or at least most) architectures because some code 
outside the kernel tree needs (or used to need?) to be able to do the 
reverse of regs_query_register_offset(). It's easy enough to remove 
this, but that will make arm64 unlike the other architectures in its 
CONFIG_HAVE_REGS_AND_STACK_ACCESS_API support.

>>>> +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr);
>>>
>>> This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we
>>> make it static and remove the declaration?
>>
>> OK.
>
> I can change it locally.

It's looking like there will need to be another iteration of the patch 
for a few small things anyway, although those changes could also be done 
as subsequent improvements.

>
> Are these going to be used in the future by uprobes?
>

It would appear not.

Thanks,
-dl

[toc] | [prev] | [next] | [standalone]

#1446510 — Re: [PATCH v15 01/10] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature

On Fri, Jul 15, 2016 at 01:51:02PM -0400, David Long wrote:
> On 07/15/2016 11:13 AM, Catalin Marinas wrote:
> >On Fri, Jul 15, 2016 at 10:51:23AM -0400, David Long wrote:
> >>On 07/15/2016 06:57 AM, Catalin Marinas wrote:
> >>>On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote:
> >>>>+extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr);
> >>>
> >>>This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we
> >>>make it static and remove the declaration?
> >>
> >>OK.
> >
> >I can change it locally.
> 
> It's looking like there will need to be another iteration of the patch for a
> few small things anyway, although those changes could also be done as
> subsequent improvements.

I've pushed the branch below with my fixups on this series. Please post
any minor changes you have as additional patches on top. Thanks.

git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/kprobes

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1443574

On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
> David A. Long (3):
>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>   arm64: Add more test functions to insn.c
>   arm64: add conditional instruction simulation support
> 
> Pratyush Anand (2):
>   arm64: Blacklist non-kprobe-able symbol
>   arm64: Treat all entry code as non-kprobe-able
> 
> Sandeepa Prabhu (4):
>   arm64: Kprobes with single stepping support
>   arm64: kprobes instruction simulation support
>   arm64: Add kernel return probes support (kretprobes)
>   kprobes: Add arm64 case in kprobe example module
> 
> William Cohen (1):
>   arm64: Add trampoline code for kretprobes

I applied these patches on top of the arm64 for-next/core branch an
tried to run the resulting kernel in a guest (on a Juno platform using
both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
the kernel fails to boot with lots of "Unexpected kernel single-step
exception at EL1".

Did you manage to run Kprobes in a guest before?

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1443608

On 07/14/2016 12:22 PM, Catalin Marinas wrote:
> On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
>> David A. Long (3):
>>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>>   arm64: Add more test functions to insn.c
>>   arm64: add conditional instruction simulation support
>>
>> Pratyush Anand (2):
>>   arm64: Blacklist non-kprobe-able symbol
>>   arm64: Treat all entry code as non-kprobe-able
>>
>> Sandeepa Prabhu (4):
>>   arm64: Kprobes with single stepping support
>>   arm64: kprobes instruction simulation support
>>   arm64: Add kernel return probes support (kretprobes)
>>   kprobes: Add arm64 case in kprobe example module
>>
>> William Cohen (1):
>>   arm64: Add trampoline code for kretprobes
> 
> I applied these patches on top of the arm64 for-next/core branch an
> tried to run the resulting kernel in a guest (on a Juno platform using
> both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
> the kernel fails to boot with lots of "Unexpected kernel single-step
> exception at EL1".
> 
> Did you manage to run Kprobes in a guest before?
> 

Hi,

I ran the systemtap testsuite several times on a physical machine running a kernel with the kprobe v15 patches without problem. Shouldn't the guest machine behave in the same manner as a host machine for single stepping and exception handling?  If the guest machine is failing, wouldn't that suggest there is a problem with the KVM handling of single stepping for guests?

-Will Cohen

[toc] | [prev] | [next] | [standalone]

#1444021

On Thu, Jul 14, 2016 at 01:09:08PM -0400, William Cohen wrote:
> On 07/14/2016 12:22 PM, Catalin Marinas wrote:
> > On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
> >> David A. Long (3):
> >>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
> >>   arm64: Add more test functions to insn.c
> >>   arm64: add conditional instruction simulation support
> >>
> >> Pratyush Anand (2):
> >>   arm64: Blacklist non-kprobe-able symbol
> >>   arm64: Treat all entry code as non-kprobe-able
> >>
> >> Sandeepa Prabhu (4):
> >>   arm64: Kprobes with single stepping support
> >>   arm64: kprobes instruction simulation support
> >>   arm64: Add kernel return probes support (kretprobes)
> >>   kprobes: Add arm64 case in kprobe example module
> >>
> >> William Cohen (1):
> >>   arm64: Add trampoline code for kretprobes
> > 
> > I applied these patches on top of the arm64 for-next/core branch an
> > tried to run the resulting kernel in a guest (on a Juno platform using
> > both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
> > the kernel fails to boot with lots of "Unexpected kernel single-step
> > exception at EL1".
> > 
> > Did you manage to run Kprobes in a guest before?
> 
> I ran the systemtap testsuite several times on a physical machine
> running a kernel with the kprobe v15 patches without problem.
> Shouldn't the guest machine behave in the same manner as a host
> machine for single stepping and exception handling?  If the guest
> machine is failing, wouldn't that suggest there is a problem with the
> KVM handling of single stepping for guests?

It didn't fail for me on the host either. What's strange is that on some
occasions even the guest managed to get to a prompt. I'll do more tests
today on different CPU configurations, just to rule out potential
hardware issues. If not hardware related, it's possible that the
interaction with KVM doesn't work as expected, maybe the
saving/restoring of the guest debug state loses information.

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1444026

On 15/07/16 08:50, Catalin Marinas wrote:
> On Thu, Jul 14, 2016 at 01:09:08PM -0400, William Cohen wrote:
>> On 07/14/2016 12:22 PM, Catalin Marinas wrote:
>>> On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
>>>> David A. Long (3):
>>>>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>>>>   arm64: Add more test functions to insn.c
>>>>   arm64: add conditional instruction simulation support
>>>>
>>>> Pratyush Anand (2):
>>>>   arm64: Blacklist non-kprobe-able symbol
>>>>   arm64: Treat all entry code as non-kprobe-able
>>>>
>>>> Sandeepa Prabhu (4):
>>>>   arm64: Kprobes with single stepping support
>>>>   arm64: kprobes instruction simulation support
>>>>   arm64: Add kernel return probes support (kretprobes)
>>>>   kprobes: Add arm64 case in kprobe example module
>>>>
>>>> William Cohen (1):
>>>>   arm64: Add trampoline code for kretprobes
>>>
>>> I applied these patches on top of the arm64 for-next/core branch an
>>> tried to run the resulting kernel in a guest (on a Juno platform using
>>> both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
>>> the kernel fails to boot with lots of "Unexpected kernel single-step
>>> exception at EL1".
>>>
>>> Did you manage to run Kprobes in a guest before?
>>
>> I ran the systemtap testsuite several times on a physical machine
>> running a kernel with the kprobe v15 patches without problem.
>> Shouldn't the guest machine behave in the same manner as a host
>> machine for single stepping and exception handling?  If the guest
>> machine is failing, wouldn't that suggest there is a problem with the
>> KVM handling of single stepping for guests?
> 
> It didn't fail for me on the host either. What's strange is that on some
> occasions even the guest managed to get to a prompt. I'll do more tests
> today on different CPU configurations, just to rule out potential
> hardware issues. If not hardware related, it's possible that the
> interaction with KVM doesn't work as expected, maybe the
> saving/restoring of the guest debug state loses information.

Could well be the latter. I'll try to have a look, but Alex Bennée (on
cc) is our man when it comes to the KVM debug infrastructure.

Alex, any chance you could try this and shed some light on it?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[toc] | [prev] | [next] | [standalone]

#1444079

Marc Zyngier <marc.zyngier@arm.com> writes:

> On 15/07/16 08:50, Catalin Marinas wrote:
>> On Thu, Jul 14, 2016 at 01:09:08PM -0400, William Cohen wrote:
>>> On 07/14/2016 12:22 PM, Catalin Marinas wrote:
>>>> On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
>>>>> David A. Long (3):
>>>>>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>>>>>   arm64: Add more test functions to insn.c
>>>>>   arm64: add conditional instruction simulation support
>>>>>
>>>>> Pratyush Anand (2):
>>>>>   arm64: Blacklist non-kprobe-able symbol
>>>>>   arm64: Treat all entry code as non-kprobe-able
>>>>>
>>>>> Sandeepa Prabhu (4):
>>>>>   arm64: Kprobes with single stepping support
>>>>>   arm64: kprobes instruction simulation support
>>>>>   arm64: Add kernel return probes support (kretprobes)
>>>>>   kprobes: Add arm64 case in kprobe example module
>>>>>
>>>>> William Cohen (1):
>>>>>   arm64: Add trampoline code for kretprobes
>>>>
>>>> I applied these patches on top of the arm64 for-next/core branch an
>>>> tried to run the resulting kernel in a guest (on a Juno platform using
>>>> both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
>>>> the kernel fails to boot with lots of "Unexpected kernel single-step
>>>> exception at EL1".
>>>>
>>>> Did you manage to run Kprobes in a guest before?
>>>
>>> I ran the systemtap testsuite several times on a physical machine
>>> running a kernel with the kprobe v15 patches without problem.
>>> Shouldn't the guest machine behave in the same manner as a host
>>> machine for single stepping and exception handling?  If the guest
>>> machine is failing, wouldn't that suggest there is a problem with the
>>> KVM handling of single stepping for guests?
>>
>> It didn't fail for me on the host either. What's strange is that on some
>> occasions even the guest managed to get to a prompt. I'll do more tests
>> today on different CPU configurations, just to rule out potential
>> hardware issues. If not hardware related, it's possible that the
>> interaction with KVM doesn't work as expected, maybe the
>> saving/restoring of the guest debug state loses information.
>
> Could well be the latter. I'll try to have a look, but Alex Bennée (on
> cc) is our man when it comes to the KVM debug infrastructure.
>
> Alex, any chance you could try this and shed some light on it?

Sure I'll have a look. There are problems with running gdb inside a
guest while trying to debug from outside associated with single-stepping
but none of this should get in the way if your not debugging the guest.

Let me get my system spun up and see if I can reproduce.

Shall I just apply this series on top of the current master?

--
Alex Bennée

[toc] | [prev] | [next] | [standalone]

#1444094

On 15/07/16 09:59, Alex Bennée wrote:
> 
> Marc Zyngier <marc.zyngier@arm.com> writes:
> 
>> On 15/07/16 08:50, Catalin Marinas wrote:
>>> On Thu, Jul 14, 2016 at 01:09:08PM -0400, William Cohen wrote:
>>>> On 07/14/2016 12:22 PM, Catalin Marinas wrote:
>>>>> On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
>>>>>> David A. Long (3):
>>>>>>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>>>>>>   arm64: Add more test functions to insn.c
>>>>>>   arm64: add conditional instruction simulation support
>>>>>>
>>>>>> Pratyush Anand (2):
>>>>>>   arm64: Blacklist non-kprobe-able symbol
>>>>>>   arm64: Treat all entry code as non-kprobe-able
>>>>>>
>>>>>> Sandeepa Prabhu (4):
>>>>>>   arm64: Kprobes with single stepping support
>>>>>>   arm64: kprobes instruction simulation support
>>>>>>   arm64: Add kernel return probes support (kretprobes)
>>>>>>   kprobes: Add arm64 case in kprobe example module
>>>>>>
>>>>>> William Cohen (1):
>>>>>>   arm64: Add trampoline code for kretprobes
>>>>>
>>>>> I applied these patches on top of the arm64 for-next/core branch an
>>>>> tried to run the resulting kernel in a guest (on a Juno platform using
>>>>> both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
>>>>> the kernel fails to boot with lots of "Unexpected kernel single-step
>>>>> exception at EL1".
>>>>>
>>>>> Did you manage to run Kprobes in a guest before?
>>>>
>>>> I ran the systemtap testsuite several times on a physical machine
>>>> running a kernel with the kprobe v15 patches without problem.
>>>> Shouldn't the guest machine behave in the same manner as a host
>>>> machine for single stepping and exception handling?  If the guest
>>>> machine is failing, wouldn't that suggest there is a problem with the
>>>> KVM handling of single stepping for guests?
>>>
>>> It didn't fail for me on the host either. What's strange is that on some
>>> occasions even the guest managed to get to a prompt. I'll do more tests
>>> today on different CPU configurations, just to rule out potential
>>> hardware issues. If not hardware related, it's possible that the
>>> interaction with KVM doesn't work as expected, maybe the
>>> saving/restoring of the guest debug state loses information.
>>
>> Could well be the latter. I'll try to have a look, but Alex Bennée (on
>> cc) is our man when it comes to the KVM debug infrastructure.
>>
>> Alex, any chance you could try this and shed some light on it?
> 
> Sure I'll have a look. There are problems with running gdb inside a
> guest while trying to debug from outside associated with single-stepping
> but none of this should get in the way if your not debugging the guest.
> 
> Let me get my system spun up and see if I can reproduce.
> 
> Shall I just apply this series on top of the current master?

I'm trying with -rc7 at the moment.

	M.
-- 
Jazz is not dead. It just smells funny...

[toc] | [prev] | [next] | [standalone]

#1444129

On 15/07/16 09:59, Alex Bennée wrote:
> 
> Marc Zyngier <marc.zyngier@arm.com> writes:
> 
>> On 15/07/16 08:50, Catalin Marinas wrote:
>>> On Thu, Jul 14, 2016 at 01:09:08PM -0400, William Cohen wrote:
>>>> On 07/14/2016 12:22 PM, Catalin Marinas wrote:
>>>>> On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
>>>>>> David A. Long (3):
>>>>>>   arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>>>>>>   arm64: Add more test functions to insn.c
>>>>>>   arm64: add conditional instruction simulation support
>>>>>>
>>>>>> Pratyush Anand (2):
>>>>>>   arm64: Blacklist non-kprobe-able symbol
>>>>>>   arm64: Treat all entry code as non-kprobe-able
>>>>>>
>>>>>> Sandeepa Prabhu (4):
>>>>>>   arm64: Kprobes with single stepping support
>>>>>>   arm64: kprobes instruction simulation support
>>>>>>   arm64: Add kernel return probes support (kretprobes)
>>>>>>   kprobes: Add arm64 case in kprobe example module
>>>>>>
>>>>>> William Cohen (1):
>>>>>>   arm64: Add trampoline code for kretprobes
>>>>>
>>>>> I applied these patches on top of the arm64 for-next/core branch an
>>>>> tried to run the resulting kernel in a guest (on a Juno platform using
>>>>> both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
>>>>> the kernel fails to boot with lots of "Unexpected kernel single-step
>>>>> exception at EL1".
>>>>>
>>>>> Did you manage to run Kprobes in a guest before?
>>>>
>>>> I ran the systemtap testsuite several times on a physical machine
>>>> running a kernel with the kprobe v15 patches without problem.
>>>> Shouldn't the guest machine behave in the same manner as a host
>>>> machine for single stepping and exception handling?  If the guest
>>>> machine is failing, wouldn't that suggest there is a problem with the
>>>> KVM handling of single stepping for guests?
>>>
>>> It didn't fail for me on the host either. What's strange is that on some
>>> occasions even the guest managed to get to a prompt. I'll do more tests
>>> today on different CPU configurations, just to rule out potential
>>> hardware issues. If not hardware related, it's possible that the
>>> interaction with KVM doesn't work as expected, maybe the
>>> saving/restoring of the guest debug state loses information.
>>
>> Could well be the latter. I'll try to have a look, but Alex Bennée (on
>> cc) is our man when it comes to the KVM debug infrastructure.
>>
>> Alex, any chance you could try this and shed some light on it?
> 
> Sure I'll have a look. There are problems with running gdb inside a
> guest while trying to debug from outside associated with single-stepping
> but none of this should get in the way if your not debugging the guest.
> 
> Let me get my system spun up and see if I can reproduce.
> 
> Shall I just apply this series on top of the current master?

I managed to reproduce it by taskset-ing 2 vcpus on the same physical
CPU, and trying a few dozen times on Juno-r1. It is not easy to trigger,
but when it happens it is quite bad.

Warning, pure speculation ahead: I suspect that we preempt a vcpu with
single-step enabled, somehow fail to clear the SS state, schedule
another vcpu that inherits that state and takes this unexpected SS
exception.

/me goes and have a look...

	M.
-- 
Jazz is not dead. It just smells funny...

[toc] | [prev] | [next] | [standalone]

#1443648

On 07/14/2016 12:22 PM, Catalin Marinas wrote:
> On Fri, Jul 08, 2016 at 12:35:44PM -0400, David Long wrote:
>> David A. Long (3):
>>    arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature
>>    arm64: Add more test functions to insn.c
>>    arm64: add conditional instruction simulation support
>>
>> Pratyush Anand (2):
>>    arm64: Blacklist non-kprobe-able symbol
>>    arm64: Treat all entry code as non-kprobe-able
>>
>> Sandeepa Prabhu (4):
>>    arm64: Kprobes with single stepping support
>>    arm64: kprobes instruction simulation support
>>    arm64: Add kernel return probes support (kretprobes)
>>    kprobes: Add arm64 case in kprobe example module
>>
>> William Cohen (1):
>>    arm64: Add trampoline code for kretprobes
>
> I applied these patches on top of the arm64 for-next/core branch an
> tried to run the resulting kernel in a guest (on a Juno platform using
> both kvmtool and qemu) with KPROBES_SANITY_TEST enabled. Unfortunately,
> the kernel fails to boot with lots of "Unexpected kernel single-step
> exception at EL1".
>
> Did you manage to run Kprobes in a guest before?
>

I have not run this code as a guest, I have only tested it natively.

-dl

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web