[PATCH v2 0/8] Rework debug exception handling code

[PATCH v2 0/8] Rework debug exception handling code

[Will Deacon]

Hi all,

This is version two of the patches I previously posted here:

  http://lists.infradead.org/pipermail/linux-arm-kernel/2019-March/636331.html

Changes since v1 include:

  * Dropped the first two patches, since they have been merged as fixed
  * Fixed break dispatching to work with comment ranges (required by KASAN)
  * Based on 5.1-rc3

I plan to queue these up for 5.2, pending any issues.

Will

--->8

Will Deacon (8):
  arm64: debug: Remove unused return value from do_debug_exception()
  arm64: debug: Rename addr parameter for non-watchpoint exception hooks
  arm64: debug: Remove meaningless comment
  arm64: debug: Separate debug hooks based on target exception level
  arm64: kprobes: Avoid calling kprobes debug handlers explicitly
  arm64: debug: Remove redundant user_mode(regs) checks from debug
    handlers
  arm64: probes: Move magic BRK values into brk-imm.h
  arm64: debug: Clean up brk_handler()

 arch/arm64/include/asm/brk-imm.h        |   5 ++
 arch/arm64/include/asm/debug-monitors.h |  25 ++++---
 arch/arm64/include/asm/esr.h            |   4 +-
 arch/arm64/include/asm/kprobes.h        |   2 -
 arch/arm64/kernel/debug-monitors.c      | 114 ++++++++++++++++++--------------
 arch/arm64/kernel/kgdb.c                |  30 +++------
 arch/arm64/kernel/probes/kprobes.c      |  22 +++---
 arch/arm64/kernel/probes/uprobes.c      |  19 ++----
 arch/arm64/kernel/traps.c               |  26 +++-----
 arch/arm64/mm/fault.c                   |  14 ++--
 10 files changed, 131 insertions(+), 130 deletions(-)

-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/8] arm64: debug: Remove unused return value from do_debug_exception()

[Will Deacon]

do_debug_exception() goes out of its way to return a value that isn't
ever used, so just make the thing void.

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/mm/fault.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 1a7e92ab69eb..606c46aeed99 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -810,13 +810,12 @@ void __init hook_debug_fault_code(int nr,
 	debug_fault_info[nr].name	= name;
 }
 
-asmlinkage int __exception do_debug_exception(unsigned long addr_if_watchpoint,
-					      unsigned int esr,
-					      struct pt_regs *regs)
+asmlinkage void __exception do_debug_exception(unsigned long addr_if_watchpoint,
+					       unsigned int esr,
+					       struct pt_regs *regs)
 {
 	const struct fault_info *inf = esr_to_debug_fault_info(esr);
 	unsigned long pc = instruction_pointer(regs);
-	int rv;
 
 	/*
 	 * Tell lockdep we disabled irqs in entry.S. Do nothing if they were
@@ -828,17 +827,12 @@ asmlinkage int __exception do_debug_exception(unsigned long addr_if_watchpoint,
 	if (user_mode(regs) && !is_ttbr0_addr(pc))
 		arm64_apply_bp_hardening();
 
-	if (!inf->fn(addr_if_watchpoint, esr, regs)) {
-		rv = 1;
-	} else {
+	if (inf->fn(addr_if_watchpoint, esr, regs)) {
 		arm64_notify_die(inf->name, regs,
 				 inf->sig, inf->code, (void __user *)pc, esr);
-		rv = 0;
 	}
 
 	if (interrupts_enabled(regs))
 		trace_hardirqs_on();
-
-	return rv;
 }
 NOKPROBE_SYMBOL(do_debug_exception);
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/8] arm64: debug: Rename addr parameter for non-watchpoint exception hooks

[Will Deacon]

Since the 'addr' parameter contains an UNKNOWN value for non-watchpoint
debug exceptions, rename it to 'unused' for those hooks so we don't get
tempted to use it in the future.

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/debug-monitors.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index d7bb6aefae0a..c4c263d0cf0f 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -222,7 +222,7 @@ static void send_user_sigtrap(int si_code)
 			     "User debug trap");
 }
 
-static int single_step_handler(unsigned long addr, unsigned int esr,
+static int single_step_handler(unsigned long unused, unsigned int esr,
 			       struct pt_regs *regs)
 {
 	bool handler_found = false;
@@ -302,7 +302,7 @@ static int call_break_hook(struct pt_regs *regs, unsigned int esr)
 }
 NOKPROBE_SYMBOL(call_break_hook);
 
-static int brk_handler(unsigned long addr, unsigned int esr,
+static int brk_handler(unsigned long unused, unsigned int esr,
 		       struct pt_regs *regs)
 {
 	bool handler_found = false;
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/8] arm64: debug: Remove meaningless comment

[Will Deacon]

The comment next to the definition of our 'break_hook' list head is
at best wrong but mainly just meaningless. Rip it out.

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/debug-monitors.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index c4c263d0cf0f..744229d10ca8 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -264,11 +264,6 @@ static int single_step_handler(unsigned long unused, unsigned int esr,
 }
 NOKPROBE_SYMBOL(single_step_handler);
 
-/*
- * Breakpoint handler is re-entrant as another breakpoint can
- * hit within breakpoint handler, especically in kprobes.
- * Use reader/writer locks instead of plain spinlock.
- */
 static LIST_HEAD(break_hook);
 static DEFINE_SPINLOCK(break_hook_lock);
 
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/8] arm64: debug: Separate debug hooks based on target exception level

[Will Deacon]

Mixing kernel and user debug hooks together is highly error-prone as it
relies on all of the hooks to figure out whether the exception came from
kernel or user, and then to act accordingly.

Make our debug hook code a little more robust by maintaining separate
hook lists for user and kernel, with separate registration functions
to force callers to be explicit about the exception levels that they
care about.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/brk-imm.h        |  1 +
 arch/arm64/include/asm/debug-monitors.h | 18 ++++---
 arch/arm64/kernel/debug-monitors.c      | 85 +++++++++++++++++++++++----------
 arch/arm64/kernel/kgdb.c                | 22 ++++-----
 arch/arm64/kernel/probes/uprobes.c      |  7 ++-
 arch/arm64/kernel/traps.c               | 20 ++++----
 6 files changed, 95 insertions(+), 58 deletions(-)

diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
index 2945fe6cd863..fec9e1384641 100644
--- a/arch/arm64/include/asm/brk-imm.h
+++ b/arch/arm64/include/asm/brk-imm.h
@@ -23,5 +23,6 @@
 #define KGDB_COMPILED_DBG_BRK_IMM	0x401
 #define BUG_BRK_IMM			0x800
 #define KASAN_BRK_IMM			0x900
+#define KASAN_BRK_MASK			0x0ff
 
 #endif
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index a44cf5225429..7d37cfa5cc16 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -94,18 +94,24 @@ struct step_hook {
 	int (*fn)(struct pt_regs *regs, unsigned int esr);
 };
 
-void register_step_hook(struct step_hook *hook);
-void unregister_step_hook(struct step_hook *hook);
+void register_user_step_hook(struct step_hook *hook);
+void unregister_user_step_hook(struct step_hook *hook);
+
+void register_kernel_step_hook(struct step_hook *hook);
+void unregister_kernel_step_hook(struct step_hook *hook);
 
 struct break_hook {
 	struct list_head node;
-	u32 esr_val;
-	u32 esr_mask;
 	int (*fn)(struct pt_regs *regs, unsigned int esr);
+	u16 imm;
+	u16 mask; /* These bits are ignored when comparing with imm */
 };
 
-void register_break_hook(struct break_hook *hook);
-void unregister_break_hook(struct break_hook *hook);
+void register_user_break_hook(struct break_hook *hook);
+void unregister_user_break_hook(struct break_hook *hook);
+
+void register_kernel_break_hook(struct break_hook *hook);
+void unregister_kernel_break_hook(struct break_hook *hook);
 
 u8 debug_monitors_arch(void);
 
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 744229d10ca8..9b3fd7fa5b43 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -163,25 +163,46 @@ static void clear_regs_spsr_ss(struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(clear_regs_spsr_ss);
 
-/* EL1 Single Step Handler hooks */
-static LIST_HEAD(step_hook);
-static DEFINE_SPINLOCK(step_hook_lock);
+static DEFINE_SPINLOCK(debug_hook_lock);
+static LIST_HEAD(user_step_hook);
+static LIST_HEAD(kernel_step_hook);
 
-void register_step_hook(struct step_hook *hook)
+static void register_debug_hook(struct list_head *node, struct list_head *list)
 {
-	spin_lock(&step_hook_lock);
-	list_add_rcu(&hook->node, &step_hook);
-	spin_unlock(&step_hook_lock);
+	spin_lock(&debug_hook_lock);
+	list_add_rcu(node, list);
+	spin_unlock(&debug_hook_lock);
+
 }
 
-void unregister_step_hook(struct step_hook *hook)
+static void unregister_debug_hook(struct list_head *node)
 {
-	spin_lock(&step_hook_lock);
-	list_del_rcu(&hook->node);
-	spin_unlock(&step_hook_lock);
+	spin_lock(&debug_hook_lock);
+	list_del_rcu(node);
+	spin_unlock(&debug_hook_lock);
 	synchronize_rcu();
 }
 
+void register_user_step_hook(struct step_hook *hook)
+{
+	register_debug_hook(&hook->node, &user_step_hook);
+}
+
+void unregister_user_step_hook(struct step_hook *hook)
+{
+	unregister_debug_hook(&hook->node);
+}
+
+void register_kernel_step_hook(struct step_hook *hook)
+{
+	register_debug_hook(&hook->node, &kernel_step_hook);
+}
+
+void unregister_kernel_step_hook(struct step_hook *hook)
+{
+	unregister_debug_hook(&hook->node);
+}
+
 /*
  * Call registered single step handlers
  * There is no Syndrome info to check for determining the handler.
@@ -191,11 +212,14 @@ void unregister_step_hook(struct step_hook *hook)
 static int call_step_hook(struct pt_regs *regs, unsigned int esr)
 {
 	struct step_hook *hook;
+	struct list_head *list;
 	int retval = DBG_HOOK_ERROR;
 
+	list = user_mode(regs) ? &user_step_hook : &kernel_step_hook;
+
 	rcu_read_lock();
 
-	list_for_each_entry_rcu(hook, &step_hook, node)	{
+	list_for_each_entry_rcu(hook, list, node)	{
 		retval = hook->fn(regs, esr);
 		if (retval == DBG_HOOK_HANDLED)
 			break;
@@ -264,33 +288,44 @@ static int single_step_handler(unsigned long unused, unsigned int esr,
 }
 NOKPROBE_SYMBOL(single_step_handler);
 
-static LIST_HEAD(break_hook);
-static DEFINE_SPINLOCK(break_hook_lock);
+static LIST_HEAD(user_break_hook);
+static LIST_HEAD(kernel_break_hook);
 
-void register_break_hook(struct break_hook *hook)
+void register_user_break_hook(struct break_hook *hook)
 {
-	spin_lock(&break_hook_lock);
-	list_add_rcu(&hook->node, &break_hook);
-	spin_unlock(&break_hook_lock);
+	register_debug_hook(&hook->node, &user_break_hook);
 }
 
-void unregister_break_hook(struct break_hook *hook)
+void unregister_user_break_hook(struct break_hook *hook)
 {
-	spin_lock(&break_hook_lock);
-	list_del_rcu(&hook->node);
-	spin_unlock(&break_hook_lock);
-	synchronize_rcu();
+	unregister_debug_hook(&hook->node);
+}
+
+void register_kernel_break_hook(struct break_hook *hook)
+{
+	register_debug_hook(&hook->node, &kernel_break_hook);
+}
+
+void unregister_kernel_break_hook(struct break_hook *hook)
+{
+	unregister_debug_hook(&hook->node);
 }
 
 static int call_break_hook(struct pt_regs *regs, unsigned int esr)
 {
 	struct break_hook *hook;
+	struct list_head *list;
 	int (*fn)(struct pt_regs *regs, unsigned int esr) = NULL;
 
+	list = user_mode(regs) ? &user_break_hook : &kernel_break_hook;
+
 	rcu_read_lock();
-	list_for_each_entry_rcu(hook, &break_hook, node)
-		if ((esr & hook->esr_mask) == hook->esr_val)
+	list_for_each_entry_rcu(hook, list, node) {
+		unsigned int comment = esr & BRK64_ESR_MASK;
+
+		if ((comment & ~hook->mask) == hook->imm)
 			fn = hook->fn;
+	}
 	rcu_read_unlock();
 
 	return fn ? fn(regs, esr) : DBG_HOOK_ERROR;
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 691854b77c7f..4c01f299aeb2 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -275,15 +275,13 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 NOKPROBE_SYMBOL(kgdb_step_brk_fn);
 
 static struct break_hook kgdb_brkpt_hook = {
-	.esr_mask	= 0xffffffff,
-	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_DYN_DBG_BRK_IMM),
-	.fn		= kgdb_brk_fn
+	.fn		= kgdb_brk_fn,
+	.imm		= KGDB_DYN_DBG_BRK_IMM,
 };
 
 static struct break_hook kgdb_compiled_brkpt_hook = {
-	.esr_mask	= 0xffffffff,
-	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_COMPILED_DBG_BRK_IMM),
-	.fn		= kgdb_compiled_brk_fn
+	.fn		= kgdb_compiled_brk_fn,
+	.imm		= KGDB_COMPILED_DBG_BRK_IMM,
 };
 
 static struct step_hook kgdb_step_hook = {
@@ -332,9 +330,9 @@ int kgdb_arch_init(void)
 	if (ret != 0)
 		return ret;
 
-	register_break_hook(&kgdb_brkpt_hook);
-	register_break_hook(&kgdb_compiled_brkpt_hook);
-	register_step_hook(&kgdb_step_hook);
+	register_kernel_break_hook(&kgdb_brkpt_hook);
+	register_kernel_break_hook(&kgdb_compiled_brkpt_hook);
+	register_kernel_step_hook(&kgdb_step_hook);
 	return 0;
 }
 
@@ -345,9 +343,9 @@ int kgdb_arch_init(void)
  */
 void kgdb_arch_exit(void)
 {
-	unregister_break_hook(&kgdb_brkpt_hook);
-	unregister_break_hook(&kgdb_compiled_brkpt_hook);
-	unregister_step_hook(&kgdb_step_hook);
+	unregister_kernel_break_hook(&kgdb_brkpt_hook);
+	unregister_kernel_break_hook(&kgdb_compiled_brkpt_hook);
+	unregister_kernel_step_hook(&kgdb_step_hook);
 	unregister_die_notifier(&kgdb_notifier);
 }
 
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index 636ca0119c0e..7d6ea88796a6 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -195,8 +195,7 @@ static int uprobe_single_step_handler(struct pt_regs *regs,
 
 /* uprobe breakpoint handler hook */
 static struct break_hook uprobes_break_hook = {
-	.esr_mask = BRK64_ESR_MASK,
-	.esr_val = BRK64_ESR_UPROBES,
+	.imm = BRK64_ESR_UPROBES,
 	.fn = uprobe_breakpoint_handler,
 };
 
@@ -207,8 +206,8 @@ static struct step_hook uprobes_step_hook = {
 
 static int __init arch_init_uprobes(void)
 {
-	register_break_hook(&uprobes_break_hook);
-	register_step_hook(&uprobes_step_hook);
+	register_user_break_hook(&uprobes_break_hook);
+	register_user_step_hook(&uprobes_step_hook);
 
 	return 0;
 }
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 8ad119c3f665..85fdc3d7c556 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -969,9 +969,8 @@ static int bug_handler(struct pt_regs *regs, unsigned int esr)
 }
 
 static struct break_hook bug_break_hook = {
-	.esr_val = 0xf2000000 | BUG_BRK_IMM,
-	.esr_mask = 0xffffffff,
 	.fn = bug_handler,
+	.imm = BUG_BRK_IMM,
 };
 
 #ifdef CONFIG_KASAN_SW_TAGS
@@ -1016,13 +1015,10 @@ static int kasan_handler(struct pt_regs *regs, unsigned int esr)
 	return DBG_HOOK_HANDLED;
 }
 
-#define KASAN_ESR_VAL (0xf2000000 | KASAN_BRK_IMM)
-#define KASAN_ESR_MASK 0xffffff00
-
 static struct break_hook kasan_break_hook = {
-	.esr_val = KASAN_ESR_VAL,
-	.esr_mask = KASAN_ESR_MASK,
-	.fn = kasan_handler,
+	.fn	= kasan_handler,
+	.imm	= KASAN_BRK_IMM,
+	.mask	= KASAN_BRK_MASK,
 };
 #endif
 
@@ -1034,7 +1030,9 @@ int __init early_brk64(unsigned long addr, unsigned int esr,
 		struct pt_regs *regs)
 {
 #ifdef CONFIG_KASAN_SW_TAGS
-	if ((esr & KASAN_ESR_MASK) == KASAN_ESR_VAL)
+	unsigned int comment = esr & BRK64_ESR_MASK;
+
+	if ((comment & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
 		return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
 #endif
 	return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
@@ -1043,8 +1041,8 @@ int __init early_brk64(unsigned long addr, unsigned int esr,
 /* This registration must happen early, before debug_traps_init(). */
 void __init trap_init(void)
 {
-	register_break_hook(&bug_break_hook);
+	register_kernel_break_hook(&bug_break_hook);
 #ifdef CONFIG_KASAN_SW_TAGS
-	register_break_hook(&kasan_break_hook);
+	register_kernel_break_hook(&kasan_break_hook);
 #endif
 }
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/8] arm64: kprobes: Avoid calling kprobes debug handlers explicitly

[Will Deacon]

Kprobes bypasses our debug hook registration code so that it doesn't
get tangled up with recursive debug exceptions from things like lockdep:

  http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324385.html

However, since then, (a) the hook list has become RCU protected and (b)
the kprobes hooks were found not to filter out exceptions from userspace
correctly. On top of that, the step handler is invoked directly from
single_step_handler(), which *does* use the debug hook list, so it's
clearly not the end of the world.

For now, have kprobes use the debug hook registration API like everybody
else. We can revisit this in the future if this is found to limit
coverage significantly.

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/kprobes.h   |  2 --
 arch/arm64/kernel/debug-monitors.c | 10 ----------
 arch/arm64/kernel/probes/kprobes.c | 16 ++++++++++++++--
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
index d5a44cf859e9..21721fbf44e7 100644
--- a/arch/arm64/include/asm/kprobes.h
+++ b/arch/arm64/include/asm/kprobes.h
@@ -54,8 +54,6 @@ 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);
 void kretprobe_trampoline(void);
 void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
 
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 9b3fd7fa5b43..f4d8cda8830d 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -258,10 +258,6 @@ static int single_step_handler(unsigned long unused, unsigned int esr,
 	if (!reinstall_suspended_bps(regs))
 		return 0;
 
-#ifdef	CONFIG_KPROBES
-	if (kprobe_single_step_handler(regs, esr) == DBG_HOOK_HANDLED)
-		handler_found = true;
-#endif
 	if (!handler_found && call_step_hook(regs, esr) == DBG_HOOK_HANDLED)
 		handler_found = true;
 
@@ -337,12 +333,6 @@ static int brk_handler(unsigned long unused, unsigned int esr,
 {
 	bool handler_found = false;
 
-#ifdef	CONFIG_KPROBES
-	if ((esr & BRK64_ESR_MASK) == BRK64_ESR_KPROBES) {
-		if (kprobe_breakpoint_handler(regs, esr) == DBG_HOOK_HANDLED)
-			handler_found = true;
-	}
-#endif
 	if (!handler_found && call_break_hook(regs, esr) == DBG_HOOK_HANDLED)
 		handler_found = true;
 
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 7a679caf4585..baf97f47aec0 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -439,7 +439,7 @@ kprobe_ss_hit(struct kprobe_ctlblk *kcb, unsigned long addr)
 	return DBG_HOOK_ERROR;
 }
 
-int __kprobes
+static int __kprobes
 kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
 {
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
@@ -461,7 +461,11 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
 	return retval;
 }
 
-int __kprobes
+static struct step_hook kprobes_step_hook = {
+	.fn = kprobe_single_step_handler,
+};
+
+static int __kprobes
 kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
 {
 	if (user_mode(regs))
@@ -471,6 +475,11 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
 	return DBG_HOOK_HANDLED;
 }
 
+static struct break_hook kprobes_break_hook = {
+	.imm = BRK64_ESR_KPROBES,
+	.fn = kprobe_breakpoint_handler,
+};
+
 /*
  * Provide a blacklist of symbols identifying ranges which cannot be kprobed.
  * This blacklist is exposed to userspace via debugfs (kprobes/blacklist).
@@ -599,5 +608,8 @@ int __kprobes arch_trampoline_kprobe(struct kprobe *p)
 
 int __init arch_init_kprobes(void)
 {
+	register_kernel_break_hook(&kprobes_break_hook);
+	register_kernel_step_hook(&kprobes_step_hook);
+
 	return 0;
 }
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/8] arm64: debug: Remove redundant user_mode(regs) checks from debug handlers

[Will Deacon]

Now that the debug hook dispatching code takes the triggering exception
level into account, there's no need for the hooks themselves to poke
around with user_mode(regs).

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/kgdb.c           |  8 +-------
 arch/arm64/kernel/probes/kprobes.c |  6 ------
 arch/arm64/kernel/probes/uprobes.c | 12 ++++--------
 arch/arm64/kernel/traps.c          |  6 ------
 4 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 4c01f299aeb2..30853d5b7859 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -244,9 +244,6 @@ int kgdb_arch_handle_exception(int exception_vector, int signo,
 
 static int kgdb_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
-	if (user_mode(regs))
-		return DBG_HOOK_ERROR;
-
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
 	return DBG_HOOK_HANDLED;
 }
@@ -254,9 +251,6 @@ NOKPROBE_SYMBOL(kgdb_brk_fn)
 
 static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
-	if (user_mode(regs))
-		return DBG_HOOK_ERROR;
-
 	compiled_break = 1;
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
 
@@ -266,7 +260,7 @@ NOKPROBE_SYMBOL(kgdb_compiled_brk_fn);
 
 static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 {
-	if (user_mode(regs) || !kgdb_single_step)
+	if (!kgdb_single_step)
 		return DBG_HOOK_ERROR;
 
 	kgdb_handle_exception(1, SIGTRAP, 0, regs);
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index baf97f47aec0..000f32d1a756 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -445,9 +445,6 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 	int retval;
 
-	if (user_mode(regs))
-		return DBG_HOOK_ERROR;
-
 	/* return error if this is not our step */
 	retval = kprobe_ss_hit(kcb, instruction_pointer(regs));
 
@@ -468,9 +465,6 @@ static struct step_hook kprobes_step_hook = {
 static int __kprobes
 kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
 {
-	if (user_mode(regs))
-		return DBG_HOOK_ERROR;
-
 	kprobe_handler(regs);
 	return DBG_HOOK_HANDLED;
 }
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index 7d6ea88796a6..f37ab9567676 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -171,7 +171,7 @@ int arch_uprobe_exception_notify(struct notifier_block *self,
 static int uprobe_breakpoint_handler(struct pt_regs *regs,
 		unsigned int esr)
 {
-	if (user_mode(regs) && uprobe_pre_sstep_notifier(regs))
+	if (uprobe_pre_sstep_notifier(regs))
 		return DBG_HOOK_HANDLED;
 
 	return DBG_HOOK_ERROR;
@@ -182,13 +182,9 @@ static int uprobe_single_step_handler(struct pt_regs *regs,
 {
 	struct uprobe_task *utask = current->utask;
 
-	if (user_mode(regs)) {
-		WARN_ON(utask &&
-			(instruction_pointer(regs) != utask->xol_vaddr + 4));
-
-		if (uprobe_post_sstep_notifier(regs))
-			return DBG_HOOK_HANDLED;
-	}
+	WARN_ON(utask && (instruction_pointer(regs) != utask->xol_vaddr + 4));
+	if (uprobe_post_sstep_notifier(regs))
+		return DBG_HOOK_HANDLED;
 
 	return DBG_HOOK_ERROR;
 }
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 85fdc3d7c556..091379744d2f 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -947,9 +947,6 @@ int is_valid_bugaddr(unsigned long addr)
 
 static int bug_handler(struct pt_regs *regs, unsigned int esr)
 {
-	if (user_mode(regs))
-		return DBG_HOOK_ERROR;
-
 	switch (report_bug(regs->pc, regs)) {
 	case BUG_TRAP_TYPE_BUG:
 		die("Oops - BUG", regs, 0);
@@ -988,9 +985,6 @@ static int kasan_handler(struct pt_regs *regs, unsigned int esr)
 	u64 addr = regs->regs[0];
 	u64 pc = regs->pc;
 
-	if (user_mode(regs))
-		return DBG_HOOK_ERROR;
-
 	kasan_report(addr, size, write, pc);
 
 	/*
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 7/8] arm64: probes: Move magic BRK values into brk-imm.h

[Will Deacon]

kprobes and uprobes reserve some BRK immediates for installing their
probes. Define these along with the other reservations in brk-imm.h
and rename the ESR definitions to be consistent with the others that we
already have.

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/brk-imm.h        | 4 ++++
 arch/arm64/include/asm/debug-monitors.h | 7 ++-----
 arch/arm64/include/asm/esr.h            | 4 +---
 arch/arm64/kernel/debug-monitors.c      | 2 +-
 arch/arm64/kernel/probes/kprobes.c      | 2 +-
 arch/arm64/kernel/probes/uprobes.c      | 2 +-
 arch/arm64/kernel/traps.c               | 2 +-
 7 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
index fec9e1384641..d84294064e6a 100644
--- a/arch/arm64/include/asm/brk-imm.h
+++ b/arch/arm64/include/asm/brk-imm.h
@@ -11,6 +11,8 @@
 
 /*
  * #imm16 values used for BRK instruction generation
+ * 0x004: for installing kprobes
+ * 0x005: for installing uprobes
  * Allowed values for kgdb are 0x400 - 0x7ff
  * 0x100: for triggering a fault on purpose (reserved)
  * 0x400: for dynamic BRK instruction
@@ -18,6 +20,8 @@
  * 0x800: kernel-mode BUG() and WARN() traps
  * 0x9xx: tag-based KASAN trap (allowed values 0x900 - 0x9ff)
  */
+#define KPROBES_BRK_IMM			0x004
+#define UPROBES_BRK_IMM			0x005
 #define FAULT_BRK_IMM			0x100
 #define KGDB_DYN_DBG_BRK_IMM		0x400
 #define KGDB_COMPILED_DBG_BRK_IMM	0x401
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 7d37cfa5cc16..0679f781696d 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -65,12 +65,9 @@
 #define CACHE_FLUSH_IS_SAFE		1
 
 /* kprobes BRK opcodes with ESR encoding  */
-#define BRK64_ESR_MASK		0xFFFF
-#define BRK64_ESR_KPROBES	0x0004
-#define BRK64_OPCODE_KPROBES	(AARCH64_BREAK_MON | (BRK64_ESR_KPROBES << 5))
+#define BRK64_OPCODE_KPROBES	(AARCH64_BREAK_MON | (KPROBES_BRK_IMM << 5))
 /* uprobes BRK opcodes with ESR encoding  */
-#define BRK64_ESR_UPROBES	0x0005
-#define BRK64_OPCODE_UPROBES	(AARCH64_BREAK_MON | (BRK64_ESR_UPROBES << 5))
+#define BRK64_OPCODE_UPROBES	(AARCH64_BREAK_MON | (UPROBES_BRK_IMM << 5))
 
 /* AArch32 */
 #define DBG_ESR_EVT_BKPT	0x4
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 52233f00d53d..3541720189c9 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -156,9 +156,7 @@
 				 ESR_ELx_WFx_ISS_WFI)
 
 /* BRK instruction trap from AArch64 state */
-#define ESR_ELx_VAL_BRK64(imm)					\
-	((ESR_ELx_EC_BRK64 << ESR_ELx_EC_SHIFT) | ESR_ELx_IL |	\
-	 ((imm) & 0xffff))
+#define ESR_ELx_BRK64_ISS_COMMENT_MASK	0xffff
 
 /* ISS field definitions for System instruction traps */
 #define ESR_ELx_SYS64_ISS_RES0_SHIFT	22
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index f4d8cda8830d..2692a0a27cf3 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -317,7 +317,7 @@ static int call_break_hook(struct pt_regs *regs, unsigned int esr)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(hook, list, node) {
-		unsigned int comment = esr & BRK64_ESR_MASK;
+		unsigned int comment = esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
 
 		if ((comment & ~hook->mask) == hook->imm)
 			fn = hook->fn;
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 000f32d1a756..2509fcb6d404 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -470,7 +470,7 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
 }
 
 static struct break_hook kprobes_break_hook = {
-	.imm = BRK64_ESR_KPROBES,
+	.imm = KPROBES_BRK_IMM,
 	.fn = kprobe_breakpoint_handler,
 };
 
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index f37ab9567676..605945eac1f8 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -191,7 +191,7 @@ static int uprobe_single_step_handler(struct pt_regs *regs,
 
 /* uprobe breakpoint handler hook */
 static struct break_hook uprobes_break_hook = {
-	.imm = BRK64_ESR_UPROBES,
+	.imm = UPROBES_BRK_IMM,
 	.fn = uprobe_breakpoint_handler,
 };
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 091379744d2f..74598396e0bf 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -1024,7 +1024,7 @@ int __init early_brk64(unsigned long addr, unsigned int esr,
 		struct pt_regs *regs)
 {
 #ifdef CONFIG_KASAN_SW_TAGS
-	unsigned int comment = esr & BRK64_ESR_MASK;
+	unsigned int comment = esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
 
 	if ((comment & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
 		return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 8/8] arm64: debug: Clean up brk_handler()

[Will Deacon]

brk_handler() now looks pretty strange and can be refactored to drop its
funny 'handler_found' local variable altogether.

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/debug-monitors.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 2692a0a27cf3..800486cc4823 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -331,14 +331,12 @@ NOKPROBE_SYMBOL(call_break_hook);
 static int brk_handler(unsigned long unused, unsigned int esr,
 		       struct pt_regs *regs)
 {
-	bool handler_found = false;
-
-	if (!handler_found && call_break_hook(regs, esr) == DBG_HOOK_HANDLED)
-		handler_found = true;
+	if (call_break_hook(regs, esr) == DBG_HOOK_HANDLED)
+		return 0;
 
-	if (!handler_found && user_mode(regs)) {
+	if (user_mode(regs)) {
 		send_user_sigtrap(TRAP_BRKPT);
-	} else if (!handler_found) {
+	} else {
 		pr_warn("Unexpected kernel BRK exception at EL1\n");
 		return -EFAULT;
 	}
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/8] arm64: debug: Separate debug hooks based on target exception level

[Mark Rutland]

On Fri, Apr 05, 2019 at 01:40:44PM +0100, Will Deacon wrote:
> Mixing kernel and user debug hooks together is highly error-prone as it
> relies on all of the hooks to figure out whether the exception came from
> kernel or user, and then to act accordingly.
> 
> Make our debug hook code a little more robust by maintaining separate
> hook lists for user and kernel, with separate registration functions
> to force callers to be explicit about the exception levels that they
> care about.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>

One minor comment below, but either way this looks good to me:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

> ---
>  arch/arm64/include/asm/brk-imm.h        |  1 +
>  arch/arm64/include/asm/debug-monitors.h | 18 ++++---
>  arch/arm64/kernel/debug-monitors.c      | 85 +++++++++++++++++++++++----------
>  arch/arm64/kernel/kgdb.c                | 22 ++++-----
>  arch/arm64/kernel/probes/uprobes.c      |  7 ++-
>  arch/arm64/kernel/traps.c               | 20 ++++----
>  6 files changed, 95 insertions(+), 58 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
> index 2945fe6cd863..fec9e1384641 100644
> --- a/arch/arm64/include/asm/brk-imm.h
> +++ b/arch/arm64/include/asm/brk-imm.h
> @@ -23,5 +23,6 @@
>  #define KGDB_COMPILED_DBG_BRK_IMM	0x401
>  #define BUG_BRK_IMM			0x800
>  #define KASAN_BRK_IMM			0x900
> +#define KASAN_BRK_MASK			0x0ff
>  
>  #endif
> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
> index a44cf5225429..7d37cfa5cc16 100644
> --- a/arch/arm64/include/asm/debug-monitors.h
> +++ b/arch/arm64/include/asm/debug-monitors.h
> @@ -94,18 +94,24 @@ struct step_hook {
>  	int (*fn)(struct pt_regs *regs, unsigned int esr);
>  };
>  
> -void register_step_hook(struct step_hook *hook);
> -void unregister_step_hook(struct step_hook *hook);
> +void register_user_step_hook(struct step_hook *hook);
> +void unregister_user_step_hook(struct step_hook *hook);
> +
> +void register_kernel_step_hook(struct step_hook *hook);
> +void unregister_kernel_step_hook(struct step_hook *hook);
>  
>  struct break_hook {
>  	struct list_head node;
> -	u32 esr_val;
> -	u32 esr_mask;
>  	int (*fn)(struct pt_regs *regs, unsigned int esr);
> +	u16 imm;
> +	u16 mask; /* These bits are ignored when comparing with imm */

It might make sense to call this imm_mask, but that's not a big deal
either way.

Mark.

>  };
>  
> -void register_break_hook(struct break_hook *hook);
> -void unregister_break_hook(struct break_hook *hook);
> +void register_user_break_hook(struct break_hook *hook);
> +void unregister_user_break_hook(struct break_hook *hook);
> +
> +void register_kernel_break_hook(struct break_hook *hook);
> +void unregister_kernel_break_hook(struct break_hook *hook);
>  
>  u8 debug_monitors_arch(void);
>  
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 744229d10ca8..9b3fd7fa5b43 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -163,25 +163,46 @@ static void clear_regs_spsr_ss(struct pt_regs *regs)
>  }
>  NOKPROBE_SYMBOL(clear_regs_spsr_ss);
>  
> -/* EL1 Single Step Handler hooks */
> -static LIST_HEAD(step_hook);
> -static DEFINE_SPINLOCK(step_hook_lock);
> +static DEFINE_SPINLOCK(debug_hook_lock);
> +static LIST_HEAD(user_step_hook);
> +static LIST_HEAD(kernel_step_hook);
>  
> -void register_step_hook(struct step_hook *hook)
> +static void register_debug_hook(struct list_head *node, struct list_head *list)
>  {
> -	spin_lock(&step_hook_lock);
> -	list_add_rcu(&hook->node, &step_hook);
> -	spin_unlock(&step_hook_lock);
> +	spin_lock(&debug_hook_lock);
> +	list_add_rcu(node, list);
> +	spin_unlock(&debug_hook_lock);
> +
>  }
>  
> -void unregister_step_hook(struct step_hook *hook)
> +static void unregister_debug_hook(struct list_head *node)
>  {
> -	spin_lock(&step_hook_lock);
> -	list_del_rcu(&hook->node);
> -	spin_unlock(&step_hook_lock);
> +	spin_lock(&debug_hook_lock);
> +	list_del_rcu(node);
> +	spin_unlock(&debug_hook_lock);
>  	synchronize_rcu();
>  }
>  
> +void register_user_step_hook(struct step_hook *hook)
> +{
> +	register_debug_hook(&hook->node, &user_step_hook);
> +}
> +
> +void unregister_user_step_hook(struct step_hook *hook)
> +{
> +	unregister_debug_hook(&hook->node);
> +}
> +
> +void register_kernel_step_hook(struct step_hook *hook)
> +{
> +	register_debug_hook(&hook->node, &kernel_step_hook);
> +}
> +
> +void unregister_kernel_step_hook(struct step_hook *hook)
> +{
> +	unregister_debug_hook(&hook->node);
> +}
> +
>  /*
>   * Call registered single step handlers
>   * There is no Syndrome info to check for determining the handler.
> @@ -191,11 +212,14 @@ void unregister_step_hook(struct step_hook *hook)
>  static int call_step_hook(struct pt_regs *regs, unsigned int esr)
>  {
>  	struct step_hook *hook;
> +	struct list_head *list;
>  	int retval = DBG_HOOK_ERROR;
>  
> +	list = user_mode(regs) ? &user_step_hook : &kernel_step_hook;
> +
>  	rcu_read_lock();
>  
> -	list_for_each_entry_rcu(hook, &step_hook, node)	{
> +	list_for_each_entry_rcu(hook, list, node)	{
>  		retval = hook->fn(regs, esr);
>  		if (retval == DBG_HOOK_HANDLED)
>  			break;
> @@ -264,33 +288,44 @@ static int single_step_handler(unsigned long unused, unsigned int esr,
>  }
>  NOKPROBE_SYMBOL(single_step_handler);
>  
> -static LIST_HEAD(break_hook);
> -static DEFINE_SPINLOCK(break_hook_lock);
> +static LIST_HEAD(user_break_hook);
> +static LIST_HEAD(kernel_break_hook);
>  
> -void register_break_hook(struct break_hook *hook)
> +void register_user_break_hook(struct break_hook *hook)
>  {
> -	spin_lock(&break_hook_lock);
> -	list_add_rcu(&hook->node, &break_hook);
> -	spin_unlock(&break_hook_lock);
> +	register_debug_hook(&hook->node, &user_break_hook);
>  }
>  
> -void unregister_break_hook(struct break_hook *hook)
> +void unregister_user_break_hook(struct break_hook *hook)
>  {
> -	spin_lock(&break_hook_lock);
> -	list_del_rcu(&hook->node);
> -	spin_unlock(&break_hook_lock);
> -	synchronize_rcu();
> +	unregister_debug_hook(&hook->node);
> +}
> +
> +void register_kernel_break_hook(struct break_hook *hook)
> +{
> +	register_debug_hook(&hook->node, &kernel_break_hook);
> +}
> +
> +void unregister_kernel_break_hook(struct break_hook *hook)
> +{
> +	unregister_debug_hook(&hook->node);
>  }
>  
>  static int call_break_hook(struct pt_regs *regs, unsigned int esr)
>  {
>  	struct break_hook *hook;
> +	struct list_head *list;
>  	int (*fn)(struct pt_regs *regs, unsigned int esr) = NULL;
>  
> +	list = user_mode(regs) ? &user_break_hook : &kernel_break_hook;
> +
>  	rcu_read_lock();
> -	list_for_each_entry_rcu(hook, &break_hook, node)
> -		if ((esr & hook->esr_mask) == hook->esr_val)
> +	list_for_each_entry_rcu(hook, list, node) {
> +		unsigned int comment = esr & BRK64_ESR_MASK;
> +
> +		if ((comment & ~hook->mask) == hook->imm)
>  			fn = hook->fn;
> +	}
>  	rcu_read_unlock();
>  
>  	return fn ? fn(regs, esr) : DBG_HOOK_ERROR;
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index 691854b77c7f..4c01f299aeb2 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -275,15 +275,13 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
>  NOKPROBE_SYMBOL(kgdb_step_brk_fn);
>  
>  static struct break_hook kgdb_brkpt_hook = {
> -	.esr_mask	= 0xffffffff,
> -	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_DYN_DBG_BRK_IMM),
> -	.fn		= kgdb_brk_fn
> +	.fn		= kgdb_brk_fn,
> +	.imm		= KGDB_DYN_DBG_BRK_IMM,
>  };
>  
>  static struct break_hook kgdb_compiled_brkpt_hook = {
> -	.esr_mask	= 0xffffffff,
> -	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_COMPILED_DBG_BRK_IMM),
> -	.fn		= kgdb_compiled_brk_fn
> +	.fn		= kgdb_compiled_brk_fn,
> +	.imm		= KGDB_COMPILED_DBG_BRK_IMM,
>  };
>  
>  static struct step_hook kgdb_step_hook = {
> @@ -332,9 +330,9 @@ int kgdb_arch_init(void)
>  	if (ret != 0)
>  		return ret;
>  
> -	register_break_hook(&kgdb_brkpt_hook);
> -	register_break_hook(&kgdb_compiled_brkpt_hook);
> -	register_step_hook(&kgdb_step_hook);
> +	register_kernel_break_hook(&kgdb_brkpt_hook);
> +	register_kernel_break_hook(&kgdb_compiled_brkpt_hook);
> +	register_kernel_step_hook(&kgdb_step_hook);
>  	return 0;
>  }
>  
> @@ -345,9 +343,9 @@ int kgdb_arch_init(void)
>   */
>  void kgdb_arch_exit(void)
>  {
> -	unregister_break_hook(&kgdb_brkpt_hook);
> -	unregister_break_hook(&kgdb_compiled_brkpt_hook);
> -	unregister_step_hook(&kgdb_step_hook);
> +	unregister_kernel_break_hook(&kgdb_brkpt_hook);
> +	unregister_kernel_break_hook(&kgdb_compiled_brkpt_hook);
> +	unregister_kernel_step_hook(&kgdb_step_hook);
>  	unregister_die_notifier(&kgdb_notifier);
>  }
>  
> diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
> index 636ca0119c0e..7d6ea88796a6 100644
> --- a/arch/arm64/kernel/probes/uprobes.c
> +++ b/arch/arm64/kernel/probes/uprobes.c
> @@ -195,8 +195,7 @@ static int uprobe_single_step_handler(struct pt_regs *regs,
>  
>  /* uprobe breakpoint handler hook */
>  static struct break_hook uprobes_break_hook = {
> -	.esr_mask = BRK64_ESR_MASK,
> -	.esr_val = BRK64_ESR_UPROBES,
> +	.imm = BRK64_ESR_UPROBES,
>  	.fn = uprobe_breakpoint_handler,
>  };
>  
> @@ -207,8 +206,8 @@ static struct step_hook uprobes_step_hook = {
>  
>  static int __init arch_init_uprobes(void)
>  {
> -	register_break_hook(&uprobes_break_hook);
> -	register_step_hook(&uprobes_step_hook);
> +	register_user_break_hook(&uprobes_break_hook);
> +	register_user_step_hook(&uprobes_step_hook);
>  
>  	return 0;
>  }
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 8ad119c3f665..85fdc3d7c556 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -969,9 +969,8 @@ static int bug_handler(struct pt_regs *regs, unsigned int esr)
>  }
>  
>  static struct break_hook bug_break_hook = {
> -	.esr_val = 0xf2000000 | BUG_BRK_IMM,
> -	.esr_mask = 0xffffffff,
>  	.fn = bug_handler,
> +	.imm = BUG_BRK_IMM,
>  };
>  
>  #ifdef CONFIG_KASAN_SW_TAGS
> @@ -1016,13 +1015,10 @@ static int kasan_handler(struct pt_regs *regs, unsigned int esr)
>  	return DBG_HOOK_HANDLED;
>  }
>  
> -#define KASAN_ESR_VAL (0xf2000000 | KASAN_BRK_IMM)
> -#define KASAN_ESR_MASK 0xffffff00
> -
>  static struct break_hook kasan_break_hook = {
> -	.esr_val = KASAN_ESR_VAL,
> -	.esr_mask = KASAN_ESR_MASK,
> -	.fn = kasan_handler,
> +	.fn	= kasan_handler,
> +	.imm	= KASAN_BRK_IMM,
> +	.mask	= KASAN_BRK_MASK,
>  };
>  #endif
>  
> @@ -1034,7 +1030,9 @@ int __init early_brk64(unsigned long addr, unsigned int esr,
>  		struct pt_regs *regs)
>  {
>  #ifdef CONFIG_KASAN_SW_TAGS
> -	if ((esr & KASAN_ESR_MASK) == KASAN_ESR_VAL)
> +	unsigned int comment = esr & BRK64_ESR_MASK;
> +
> +	if ((comment & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
>  		return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
>  #endif
>  	return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
> @@ -1043,8 +1041,8 @@ int __init early_brk64(unsigned long addr, unsigned int esr,
>  /* This registration must happen early, before debug_traps_init(). */
>  void __init trap_init(void)
>  {
> -	register_break_hook(&bug_break_hook);
> +	register_kernel_break_hook(&bug_break_hook);
>  #ifdef CONFIG_KASAN_SW_TAGS
> -	register_break_hook(&kasan_break_hook);
> +	register_kernel_break_hook(&kasan_break_hook);
>  #endif
>  }
> -- 
> 2.11.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel