arm64: some symbols blacklisted for kprobing

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

Quoted 2 lines · Pratyush Anand

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

It's a shame this has to be so manual, but I suppose it's done on a best-effort basis to catch broken probe placement.

If we miss a function and somebody probes it, do we just get stuck in a recursive exception, or could we print something suggesting that a symbol be annotated as NOKPROBE?

Quoted 10 lines · Will Deacon

Quoted 2 lines · Pratyush Anand

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

It's a shame this has to be so manual, but I suppose it's done on a best-effort basis to catch broken probe placement.

If we miss a function and somebody probes it, do we just get stuck in a recursive exception, or could we print something suggesting that a symbol be annotated as NOKPROBE?

In some cases we land into a recursive reenter_kprobe:

echo "p kfree" > /sys/kernel/debug/tracing/kprobe_events echo "p single_step_handler" >> /sys/kernel/debug/tracing/kprobe_events echo 1 > /sys/kernel/debug/tracing/events/enable

[  116.904194] BUG: failure at

.../arch/arm64/kernel/kprobes.c:288/reenter_kprobe()!

In some other
echo "p kfree" > /sys/kernel/debug/tracing/kprobe_events echo "p el0_sync" >> /sys/kernel/debug/tracing/kprobe_events echo 1 > /sys/kernel/debug/tracing/events/enable

Infinite loop of:

[  142.731336] Unexpected kernel single-step exception at EL1

In 1st case currently only address is printed. pr_warn("Unrecoverable kprobe detected at %p.\n", p->addr); So, while in 1st case we may also print name of symbol, we can not do much in second case.

Now, I am running some test with all the symbols in /proc/kallsyms and I noticed that there might be few more symbols which may not allow kprobing. So, may be I will resend this series with updates.

Quoted 2 lines · Pratyush Anand

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

It's a shame this has to be so manual, but I suppose it's done on a best-effort basis to catch broken probe placement.

If we miss a function and somebody probes it, do we just get stuck in a recursive exception, or could we print something suggesting that a symbol be annotated as NOKPROBE?

Quoted 6 lines · Will Deacon

Quoted 2 lines · Pratyush Anand

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

It's a shame this has to be so manual, but I suppose it's done on a best-effort basis to catch broken probe placement.

Ah, yes. You can use kprobe-tracer in ftrace by feeding all symbols to them and enabling it (and pray).

Quoted 5 lines

If we miss a function and somebody probes it, do we just get stuck in a recursive exception, or could we print something suggesting that a symbol be annotated as NOKPROBE?

For some cases we can detect recursion, but usually, it may reset itself or infinite recursion loop before detecting it. :(

Thank you,

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

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/arm64/kernel/debug-monitors.c | 18 ++++++++++++++++++
 arch/arm64/kernel/hw_breakpoint.c  |  8 ++++++++
 arch/arm64/kernel/kgdb.c           |  4 ++++
 arch/arm64/mm/fault.c              |  1 +
 4 files changed, 31 insertions(+)

arch/arm64/kernel/debug-monitors.c +18 -0

index 237a21f675fd..6d356b2cc674 100644
@@ -22,6 +22,7 @@
 #include <linux/debugfs.h>
 #include <linux/hardirq.h>
 #include <linux/init.h>
+#include <linux/kprobes.h>
 #include <linux/ptrace.h>
 #include <linux/kprobes.h>
 #include <linux/stat.h>

@@ -47,6 +48,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)
 {

@@ -54,6 +56,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.

@@ -102,6 +105,7 @@ void enable_debug_monitors(enum debug_elx el)
 		mdscr_write(mdscr);
 	}
 }
+NOKPROBE_SYMBOL(enable_debug_monitors);
 
 void disable_debug_monitors(enum debug_elx el)
 {

@@ -122,6 +126,7 @@ void disable_debug_monitors(enum debug_elx el)
 		mdscr_write(mdscr);
 	}
 }
+NOKPROBE_SYMBOL(disable_debug_monitors);
 
 /*
  * OS lock clearing.

@@ -173,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)
 {

@@ -182,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);

@@ -224,6 +231,7 @@ static int call_step_hook(struct pt_regs *regs, unsigned int esr)
 
 	return retval;
 }
+NOKPROBE_SYMBOL(call_step_hook);
 
 static int single_step_handler(unsigned long addr, unsigned int esr,
 			       struct pt_regs *regs)

@@ -270,6 +278,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

@@ -306,6 +315,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)

@@ -338,6 +348,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)
 {

@@ -382,6 +393,7 @@ int aarch32_break_handler(struct pt_regs *regs)
 	force_sig_info(SIGTRAP, &info, current);
 	return 0;
 }
+NOKPROBE_SYMBOL(aarch32_break_handler);
 
 static int __init debug_traps_init(void)
 {

@@ -403,6 +415,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)
 {

@@ -418,6 +431,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)
 {

@@ -425,12 +439,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)

@@ -438,8 +454,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);

arch/arm64/kernel/hw_breakpoint.c +8 -0

index 43b74a3ddaef..91b4c418abcb 100644
@@ -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>

@@ -139,6 +140,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)
 {

@@ -152,6 +154,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

@@ -169,6 +172,7 @@ static enum debug_elx debug_exception_level(int privilege)
 		return -EINVAL;
 	}
 }
+NOKPROBE_SYMBOL(debug_exception_level);
 
 enum hw_breakpoint_ops {
 	HW_BREAKPOINT_INSTALL,

@@ -573,6 +577,7 @@ static void toggle_bp_registers(int reg, enum debug_elx el, int enable)
 		write_wb_reg(reg, i, ctrl);
 	}
 }
+NOKPROBE_SYMBOL(toggle_bp_registers);
 
 /*
  * Debug exception handlers.

@@ -652,6 +657,7 @@ unlock:
 
 	return 0;
 }
+NOKPROBE_SYMBOL(breakpoint_handler);
 
 static int watchpoint_handler(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)

@@ -754,6 +760,7 @@ unlock:
 
 	return 0;
 }
+NOKPROBE_SYMBOL(watchpoint_handler);
 
 /*
  * Handle single-step exception.

@@ -811,6 +818,7 @@ int reinstall_suspended_bps(struct pt_regs *regs)
 
 	return !handled_exception;
 }
+NOKPROBE_SYMBOL(reinstall_suspended_bps);
 
 /*
  * Context-switcher for restoring suspended breakpoints.

arch/arm64/kernel/kgdb.c +4 -0

index 9469465a5e03..faa4d442688c 100644
@@ -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] = {

@@ -218,6 +219,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)
 {

@@ -226,6 +228,7 @@ 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)
 {

@@ -235,6 +238,7 @@ 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,

arch/arm64/mm/fault.c +1 -0

index 75750a91907a..0d0b02ce2074 100644
@@ -561,3 +561,4 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
 
 	return 0;
 }
+NOKPROBE_SYMBOL(do_debug_exception)

Quoted 2 lines · Pratyush Anand

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

Could you tell me how you checked that? from the code?

Quoted 5 lines · Masami Hiramatsu

Quoted 2 lines · Pratyush Anand

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

Could you tell me how you checked that? from the code?

Well.. I found out that some of the symbol like single_step_handler does not allow kprobing, and then it seemed logical to me that we should not allow kprobing of any symbols which are called in the path of do_debug_exception. So, manually :( I reviewed the code and put NOKPROBE_SYMBOL across all those.

However, now I am doing some more tests and as I said in previous reply, there are still few symbols like (_mcount) which is creating problem with following simple test and I need to look into that. In case of _mcount, I do not see any print and its complete freeze.

Entry symbols are not kprobe safe. So blacklist them for kprobing.

Signed-off-by Pratyush Anand <[email protected]>

---
 arch/arm64/kernel/entry.S       | 3 +++
 arch/arm64/kernel/kprobes.c     | 9 +++++++++
 arch/arm64/kernel/vmlinux.lds.S | 1 +
 3 files changed, 13 insertions(+)

arch/arm64/kernel/entry.S +3 -0

index a7691a378668..2ea24f6bc06b 100644
@@ -202,6 +202,7 @@ tsk	.req	x28		// current thread_info
  * Exception vectors.
  */
 
+	.section ".entry.text", "ax"
 	.align	11
 ENTRY(vectors)
 	ventry	el1_sync_invalid		// Synchronous EL1t

@@ -737,3 +738,5 @@ ENTRY(sys_rt_sigreturn_wrapper)
 	mov	x0, sp
 	b	sys_rt_sigreturn
 ENDPROC(sys_rt_sigreturn_wrapper)
+
+	.section ".text", "ax"

arch/arm64/kernel/kprobes.c +9 -0

index 6c9f8b5f04ce..9bc02c151f7f 100644
@@ -28,6 +28,7 @@
 #include <asm/debug-monitors.h>
 #include <asm/system_misc.h>
 #include <asm/insn.h>
+#include <asm-generic/sections.h>
 
 #include "kprobes.h"
 #include "kprobes-arm64.h"

@@ -661,6 +662,14 @@ int __kprobes arch_trampoline_kprobe(struct kprobe *p)
 	return 0;
 }
 
+bool arch_within_kprobe_blacklist(unsigned long addr)
+{
+	return  (addr >= (unsigned long)__kprobes_text_start &&
+		 addr < (unsigned long)__kprobes_text_end) ||
+		(addr >= (unsigned long)__entry_text_start &&
+		 addr < (unsigned long)__entry_text_end);
+}
+
 int __init arch_init_kprobes(void)
 {
 	return 0;

arch/arm64/kernel/vmlinux.lds.S +1 -0

index 1fa6adc7aa83..11fb2b0117d0 100644
@@ -97,6 +97,7 @@ SECTIONS
 			*(.exception.text)
 			__exception_text_end = .;
 			IRQENTRY_TEXT
+			ENTRY_TEXT
 			TEXT_TEXT
 			SCHED_TEXT
 			LOCK_TEXT

Quoted 1 line · Pratyush Anand

Entry symbols are not kprobe safe. So blacklist them for kprobing.

As I've said, you can also use _ASM_NOKPROBE().

This patch itself looks good to me, but I'd like to ask arm64 maintainers' opinion, whether they accept introducing entry-text section only for this purpose or not.

Reviewed-by Masami Hiramatsu <[email protected]>