* [PATCH 0/2] arm64: some symbols blacklisted for kprobing @ 2015-07-06 5:03 Pratyush Anand 2015-07-06 5:03 ` [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols Pratyush Anand 2015-07-06 5:03 ` [PATCH 2/2] arm64: Make all entry code as non-kprobe-able Pratyush Anand 0 siblings, 2 replies; 13+ messages in thread From: Pratyush Anand @ 2015-07-06 5:03 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 5:03 [PATCH 0/2] arm64: some symbols blacklisted for kprobing Pratyush Anand @ 2015-07-06 5:03 ` Pratyush Anand 2015-07-06 9:03 ` Will Deacon 2015-07-06 11:11 ` Masami Hiramatsu 2015-07-06 5:03 ` [PATCH 2/2] arm64: Make all entry code as non-kprobe-able Pratyush Anand 1 sibling, 2 replies; 13+ messages in thread From: Pratyush Anand @ 2015-07-06 5:03 UTC (permalink / raw) To: linux-arm-kernel 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> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 5:03 ` [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols Pratyush Anand @ 2015-07-06 9:03 ` Will Deacon 2015-07-06 10:48 ` Pratyush Anand 2015-07-06 11:37 ` Masami Hiramatsu 2015-07-06 11:11 ` Masami Hiramatsu 1 sibling, 2 replies; 13+ messages in thread From: Will Deacon @ 2015-07-06 9:03 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 9:03 ` Will Deacon @ 2015-07-06 10:48 ` Pratyush Anand 2015-07-06 11:56 ` Masami Hiramatsu 2015-07-06 11:37 ` Masami Hiramatsu 1 sibling, 1 reply; 13+ messages in thread From: Pratyush Anand @ 2015-07-06 10:48 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 10:48 ` Pratyush Anand @ 2015-07-06 11:56 ` Masami Hiramatsu 0 siblings, 0 replies; 13+ messages in thread From: Masami Hiramatsu @ 2015-07-06 11:56 UTC (permalink / raw) To: linux-arm-kernel 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: masami.hiramatsu.pt at hitachi.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 9:03 ` Will Deacon 2015-07-06 10:48 ` Pratyush Anand @ 2015-07-06 11:37 ` Masami Hiramatsu 2015-07-06 14:42 ` William Cohen 1 sibling, 1 reply; 13+ messages in thread From: Masami Hiramatsu @ 2015-07-06 11:37 UTC (permalink / raw) To: linux-arm-kernel 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: masami.hiramatsu.pt at hitachi.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 11:37 ` Masami Hiramatsu @ 2015-07-06 14:42 ` William Cohen 0 siblings, 0 replies; 13+ messages in thread From: William Cohen @ 2015-07-06 14:42 UTC (permalink / raw) To: linux-arm-kernel 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, > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 5:03 ` [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols Pratyush Anand 2015-07-06 9:03 ` Will Deacon @ 2015-07-06 11:11 ` Masami Hiramatsu 2015-07-06 11:49 ` Pratyush Anand 1 sibling, 1 reply; 13+ messages in thread From: Masami Hiramatsu @ 2015-07-06 11:11 UTC (permalink / raw) To: linux-arm-kernel 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 <panand@redhat.com> > --- > 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: masami.hiramatsu.pt at hitachi.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 11:11 ` Masami Hiramatsu @ 2015-07-06 11:49 ` Pratyush Anand 2015-07-09 13:53 ` Pratyush Anand 0 siblings, 1 reply; 13+ messages in thread From: Pratyush Anand @ 2015-07-06 11:49 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols 2015-07-06 11:49 ` Pratyush Anand @ 2015-07-09 13:53 ` Pratyush Anand 0 siblings, 0 replies; 13+ messages in thread From: Pratyush Anand @ 2015-07-09 13:53 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] arm64: Make all entry code as non-kprobe-able 2015-07-06 5:03 [PATCH 0/2] arm64: some symbols blacklisted for kprobing Pratyush Anand 2015-07-06 5:03 ` [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols Pratyush Anand @ 2015-07-06 5:03 ` Pratyush Anand 2015-07-06 11:03 ` Masami Hiramatsu 1 sibling, 1 reply; 13+ messages in thread From: Pratyush Anand @ 2015-07-06 5:03 UTC (permalink / raw) To: linux-arm-kernel Entry symbols are not kprobe safe. So blacklist them for kprobing. Signed-off-by: Pratyush Anand <panand@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] arm64: Make all entry code as non-kprobe-able 2015-07-06 5:03 ` [PATCH 2/2] arm64: Make all entry code as non-kprobe-able Pratyush Anand @ 2015-07-06 11:03 ` Masami Hiramatsu 2015-07-06 11:54 ` Pratyush Anand 0 siblings, 1 reply; 13+ messages in thread From: Masami Hiramatsu @ 2015-07-06 11:03 UTC (permalink / raw) To: linux-arm-kernel 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 <masami.hiramatsu.pt@hitachi.com> Thank you, > > Signed-off-by: Pratyush Anand <panand@redhat.com> > --- > 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: masami.hiramatsu.pt at hitachi.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] arm64: Make all entry code as non-kprobe-able 2015-07-06 11:03 ` Masami Hiramatsu @ 2015-07-06 11:54 ` Pratyush Anand 0 siblings, 0 replies; 13+ messages in thread From: Pratyush Anand @ 2015-07-06 11:54 UTC (permalink / raw) To: linux-arm-kernel 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 <masami.hiramatsu.pt@hitachi.com> Thanks. ~Pratyush ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-07-09 13:53 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-06 5:03 [PATCH 0/2] arm64: some symbols blacklisted for kprobing Pratyush Anand 2015-07-06 5:03 ` [PATCH 1/2] arm64: Blacklist non-kprobe-able symbols Pratyush Anand 2015-07-06 9:03 ` Will Deacon 2015-07-06 10:48 ` Pratyush Anand 2015-07-06 11:56 ` Masami Hiramatsu 2015-07-06 11:37 ` Masami Hiramatsu 2015-07-06 14:42 ` William Cohen 2015-07-06 11:11 ` Masami Hiramatsu 2015-07-06 11:49 ` Pratyush Anand 2015-07-09 13:53 ` Pratyush Anand 2015-07-06 5:03 ` [PATCH 2/2] arm64: Make all entry code as non-kprobe-able Pratyush Anand 2015-07-06 11:03 ` Masami Hiramatsu 2015-07-06 11:54 ` Pratyush Anand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).