Functions which are called from do_debug_execptions and all entry routines
must not allow to insert kprobe in it, otherwise we may witness a system hang.
This patch set blacklist such symbols.
Patches should be applied on top of arm64 kprobe patches [1].
[1] https://lkml.org/lkml/2015/6/15/514
Pratyush Anand (2):
arm64: Blacklist non-kprobe-able symbols
arm64: Make all entry code as non-kprobe-able
arch/arm64/kernel/debug-monitors.c | 18 ++++++++++++++++++
arch/arm64/kernel/entry.S | 3 +++
arch/arm64/kernel/hw_breakpoint.c | 8 ++++++++
arch/arm64/kernel/kgdb.c | 4 ++++
arch/arm64/kernel/kprobes.c | 9 +++++++++
arch/arm64/kernel/vmlinux.lds.S | 1 +
arch/arm64/mm/fault.c | 1 +
7 files changed, 44 insertions(+)
--
2.4.3
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(+)
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 237a21f675fd..6d356b2cc674 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -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);
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 43b74a3ddaef..91b4c418abcb 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>
@@ -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.
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 9469465a5e03..faa4d442688c 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] = {
@@ -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,
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 75750a91907a..0d0b02ce2074 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -561,3 +561,4 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
return 0;
}
+NOKPROBE_SYMBOL(do_debug_exception)
--
2.4.3
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(+)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index a7691a378668..2ea24f6bc06b 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -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"
diff --git a/arch/arm64/kernel/kprobes.c b/arch/arm64/kernel/kprobes.c
index 6c9f8b5f04ce..9bc02c151f7f 100644
--- a/arch/arm64/kernel/kprobes.c
+++ b/arch/arm64/kernel/kprobes.c
@@ -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;
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 1fa6adc7aa83..11fb2b0117d0 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -97,6 +97,7 @@ SECTIONS
*(.exception.text)
__exception_text_end = .;
IRQENTRY_TEXT
+ ENTRY_TEXT
TEXT_TEXT
SCHED_TEXT
LOCK_TEXT
--
2.4.3
On Mon, Jul 06, 2015 at 06:03:21AM +0100, Pratyush Anand wrote:
> 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?
Will
On 06/07/2015:10:03:24 AM, Will Deacon wrote:
> On Mon, Jul 06, 2015 at 06:03:21AM +0100, Pratyush Anand wrote:
> > 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.
~Pratyush
On 2015/07/06 14:03, Pratyush Anand wrote:
> 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]>
Thank you,
>
> 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(+)
>
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index a7691a378668..2ea24f6bc06b 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -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"
> diff --git a/arch/arm64/kernel/kprobes.c b/arch/arm64/kernel/kprobes.c
> index 6c9f8b5f04ce..9bc02c151f7f 100644
> --- a/arch/arm64/kernel/kprobes.c
> +++ b/arch/arm64/kernel/kprobes.c
> @@ -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;
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 1fa6adc7aa83..11fb2b0117d0 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -97,6 +97,7 @@ SECTIONS
> *(.exception.text)
> __exception_text_end = .;
> IRQENTRY_TEXT
> + ENTRY_TEXT
> TEXT_TEXT
> SCHED_TEXT
> LOCK_TEXT
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
On 2015/07/06 14:03, Pratyush Anand wrote:
> 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?
Thank you,
>
> 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(+)
>
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 237a21f675fd..6d356b2cc674 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -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);
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index 43b74a3ddaef..91b4c418abcb 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>
> @@ -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.
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index 9469465a5e03..faa4d442688c 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] = {
> @@ -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,
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 75750a91907a..0d0b02ce2074 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -561,3 +561,4 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
>
> return 0;
> }
> +NOKPROBE_SYMBOL(do_debug_exception)
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
On 2015/07/06 18:03, Will Deacon wrote:
> On Mon, Jul 06, 2015 at 06:03:21AM +0100, Pratyush Anand wrote:
>> 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).
> 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,
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
On 06/07/2015:08:11:19 PM, Masami Hiramatsu wrote:
> On 2015/07/06 14:03, Pratyush Anand wrote:
> > 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.
#!/bin/sh
grep ' [tT] ' /proc/kallsyms | fgrep -v '[' | awk '{print $3}' > syms.list
count=0
for i in `cat syms.list`;
do
if [ $count == 0 ]
then
echo 0 > /sys/kernel/debug/tracing/events/enable
echo > /sys/kernel/debug/tracing/kprobe_events
cat /sys/kernel/debug/tracing/kprobe_events
fi
count=`expr $count + 1`;
echo "p $i" >> /sys/kernel/debug/tracing/kprobe_events ;
echo $i $count;
if [ $count == 100 ]
then
cat /sys/kernel/debug/tracing/kprobe_events
echo 1 > /sys/kernel/debug/tracing/events/enable
sleep 1
cat /sys/kernel/debug/tracing/trace
count=0
fi
done
I understand that above test does not make sure that kprobed function
is called during test. Since at present this basic test is not
running, so I am not doing anything to exercise more and more kernel
paths. May be I will do that as second step..In fact at present, no
idea how can be done that extensively.
~Pratyush
On 06/07/2015:08:03:17 PM, Masami Hiramatsu wrote:
> On 2015/07/06 14:03, Pratyush Anand wrote:
> > Entry symbols are not kprobe safe. So blacklist them for kprobing.
>
> As I've said, you can also use _ASM_NOKPROBE().
Thanks for this info.. May be we can use it if we find some single
discrete asm routines. Since here all the entry routines need to be
blacklisted, so may be this approach should be fine. This approach
have been emulated from x86 entry code blacklist.
> 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]>
Thanks.
~Pratyush
On 2015/07/06 19:48, Pratyush Anand wrote:
> On 06/07/2015:10:03:24 AM, Will Deacon wrote:
>> On Mon, Jul 06, 2015 at 06:03:21AM +0100, Pratyush Anand wrote:
>>> 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.
Ah, that's a good point of aarch64 :)
> 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.
Sounds good to me :)
Thank you for testing!
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]
On 07/06/2015 07:37 AM, Masami Hiramatsu wrote:
> On 2015/07/06 18:03, Will Deacon wrote:
>> On Mon, Jul 06, 2015 at 06:03:21AM +0100, Pratyush Anand wrote:
>>> 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).
Yes, this approach isn't guaranteed to find problem functions. There could be problems hidden on conditional paths that are rarely called. This was observed with the aarch64 kretprobe trampoline handler. Most times it wouldn't call kfree and everything would run fine. However, sometimes the kfree which was instrumented for a systemtap test would get called from inside the trampoline handler and trigger a recursive exception. This is why aarch64 tramopline was rewritten avoid using kprobe.
Is there some tool that can generate a static call graph of the kernel? Maybe use the output of that to get more complete list of which functions should be off limits for kprobes. One complication is that some tools might not analyze functions written in assembly and miss some possible function calls.
-Will Cohen
>
>
>> 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,
>
On 06/07/2015:05:19:20 PM, Pratyush Anand wrote:
> On 06/07/2015:08:11:19 PM, Masami Hiramatsu wrote:
> > On 2015/07/06 14:03, Pratyush Anand wrote:
> > > 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.
>
Once these two patches are applied, I do not see any issue (at least)
in enabling kprobes for all the symbols of /proc/kallsyms, except
_mcount. Blacklisting _mcount seems reasonable to me, as this is
called from every function and so from do_debug_exception as well.
There might still be some path which can create issue if a kprobe is
inserted there, but I do not see any way to find them. So,
I will send V2 with following updates.Please let me know if there is
any other concern.
--- a/arch/arm64/kernel/arm64ksyms.c
+++ b/arch/arm64/kernel/arm64ksyms.c
@@ -26,6 +26,7 @@
#include <linux/syscalls.h>
#include <linux/uaccess.h>
#include <linux/io.h>
+#include <linux/kprobes.h>
#include <asm/checksum.h>
@@ -64,4 +65,5 @@ EXPORT_SYMBOL(test_and_change_bit);
#ifdef CONFIG_FUNCTION_TRACER
EXPORT_SYMBOL(_mcount);
+NOKPROBE_SYMBOL(_mcount);
#endif
~Pratyush
PS:
Some related details are here:
http://marc.info/?l=linux-kernel&m=143644472722751