Hi,
Here is the v3 series of kprobes blacklist bugfix and improvements mainly
on x86 (since I started testing on qemu-x86).
This version is just rebased on top of -tip master branch and
add bsearch nokprobe patch by Andrea (Thanks!)
This has been started from discussion about KPROBE_ENENTS_ON_NOTRACE
configuration. I tried to find notrace functions which can cause kernel
crash with kprobes using following script.
====
#!/bin/sh
i=0;
cat notrace_functions | while read f ; do
if echo p:event$i $f >> /sys/kernel/debug/tracing/kprobe_events; then
echo "Probing on $f"
echo 1 > /sys/kernel/debug/tracing/events/kprobes/event$i/enable
fi
i=$((i+1))
done
====
And I found several functions which must be blacklisted.
- optprobe template code, which is just a template code and
never be executed. Moreover, since it can be copied and
reused, if we probe it, it modifies the template code and
can cause a crash. ([1/10][2/10])
- functions which is called before kprobe_int3_handler()
handles kprobes. This can cause a breakpoint recursion. ([3/10])
- IRQ entry text, which should not be probed since register/pagetable
status has not been stable at that point. ([4/10])
- Suffixed symbols, like .constprop, .part etc. Those suffixed
symbols never be blacklisted even if the non-suffixed version
has been blacklisted. ([5/10])
- hardirq tracer also works before int3 handling. ([6/10])
- preempt_check debug function also is involved in int3 handling.
([7/10])
- RCU debug routine is also called before kprobe_int3_handler().
([8/10])
- Some lockdep functions are also involved in int3 handling.
([9/10])
- bsearch() is involved in int3 handling because of ftrace
is using it. ([10/10])
Of course there still may be some functions which can be called
by configuration change, I'll continue to test it.
Thank you,
---
Andrea Righi (1):
kprobes: Prohibit probing on bsearch()
Masami Hiramatsu (9):
x86/kprobes: Prohibit probing on optprobe template code
x86/kprobes: Move trampoline code into RODATA
x86/kprobes: Prohibit probing on functions before kprobe_int3_handler()
x86/kprobes: Prohibit probing on IRQ handlers directly
kprobes: Search non-suffixed symbol in blacklist
kprobes: Prohibit probing on hardirq tracers
kprobes: Prohibit probing on preempt_check debug functions
kprobes: Prohibit probing on RCU debug routine
kprobes: Prohibit probing on lockdep functions
arch/x86/kernel/alternative.c | 3 ++-
arch/x86/kernel/ftrace.c | 3 ++-
arch/x86/kernel/kprobes/core.c | 7 +++++++
arch/x86/kernel/kprobes/opt.c | 4 ++--
arch/x86/kernel/traps.c | 1 +
kernel/kprobes.c | 21 ++++++++++++++++++++-
kernel/locking/lockdep.c | 7 ++++++-
kernel/rcu/tree.c | 2 ++
kernel/rcu/update.c | 2 ++
kernel/trace/trace_irqsoff.c | 9 +++++++--
kernel/trace/trace_preemptirq.c | 5 +++++
lib/bsearch.c | 2 ++
lib/smp_processor_id.c | 7 +++++--
13 files changed, 63 insertions(+), 10 deletions(-)
--
Masami Hiramatsu (Linaro) <[email protected]>
Prohibit probing on optprobe template code, since it is not
a code but a template instruction sequence. If we modify
this template, copied template must be broken.
Signed-off-by: Masami Hiramatsu <[email protected]>
Fixes: 9326638cbee2 ("kprobes, x86: Use NOKPROBE_SYMBOL() instead of __kprobes annotation")
Cc: [email protected]
---
arch/x86/kernel/kprobes/opt.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 6adf6e6c2933..544bd41a514c 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -141,6 +141,11 @@ asm (
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
+NOKPROBE_SYMBOL(optprobe_template_func);
+NOKPROBE_SYMBOL(optprobe_template_entry);
+NOKPROBE_SYMBOL(optprobe_template_val);
+NOKPROBE_SYMBOL(optprobe_template_call);
+NOKPROBE_SYMBOL(optprobe_template_end);
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
Move optprobe trampoline code into RODATA since it is
not executed, but copied and modified to be used on
a trampoline buffer.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/x86/kernel/kprobes/opt.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 544bd41a514c..f14262952015 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -97,6 +97,7 @@ static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
}
asm (
+ ".pushsection .rodata\n"
"optprobe_template_func:\n"
".global optprobe_template_entry\n"
"optprobe_template_entry:\n"
@@ -136,16 +137,10 @@ asm (
#endif
".global optprobe_template_end\n"
"optprobe_template_end:\n"
- ".type optprobe_template_func, @function\n"
- ".size optprobe_template_func, .-optprobe_template_func\n");
+ ".popsection\n");
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
-NOKPROBE_SYMBOL(optprobe_template_func);
-NOKPROBE_SYMBOL(optprobe_template_entry);
-NOKPROBE_SYMBOL(optprobe_template_val);
-NOKPROBE_SYMBOL(optprobe_template_call);
-NOKPROBE_SYMBOL(optprobe_template_end);
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
Prohibit probing on IRQ handlers in irqentry_text because
if it interrupts user mode, at that point we haven't changed
to kernel space yet and which eventually leads a double fault.
E.g.
# echo p apic_timer_interrupt > kprobe_events
# echo 1 > events/kprobes/enable
PANIC: double fault, error_code: 0x0
CPU: 1 PID: 814 Comm: less Not tainted 4.20.0-rc3+ #30
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
RIP: 0010:error_entry+0x12/0xf0
[snip]
Call Trace:
<ENTRY_TRAMPOLINE>
? native_iret+0x7/0x7
? async_page_fault+0x8/0x30
? trace_hardirqs_on_thunk+0x1c/0x1c
? error_entry+0x7c/0xf0
? async_page_fault+0x8/0x30
? native_iret+0x7/0x7
? int3+0xa/0x20
? trace_hardirqs_on_thunk+0x1c/0x1c
? error_entry+0x7c/0xf0
? int3+0xa/0x20
? apic_timer_interrupt+0x1/0x20
</ENTRY_TRAMPOLINE>
Kernel panic - not syncing: Machine halted.
Kernel Offset: disabled
---[ end Kernel panic - not syncing: Machine halted. ]---
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/x86/kernel/kprobes/core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 4ba75afba527..a034cb808e7e 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1028,6 +1028,13 @@ NOKPROBE_SYMBOL(kprobe_fault_handler);
int __init arch_populate_kprobe_blacklist(void)
{
+ int ret;
+
+ ret = kprobe_add_area_blacklist((unsigned long)__irqentry_text_start,
+ (unsigned long)__irqentry_text_end);
+ if (ret)
+ return ret;
+
return kprobe_add_area_blacklist((unsigned long)__entry_text_start,
(unsigned long)__entry_text_end);
}
Newer gcc can generate some different instances of a function
with suffixed symbols if the function is optimized and only
has a part of that. (e.g. .constprop, .part etc.)
In this case, it is not enough to check the entry of kprobe
blacklist because it only records non-suffixed symbol address.
To fix this issue, search non-suffixed symbol in blacklist if
given address is within a symbol which has a suffix.
Note that this can cause false positive cases if a kprobe-safe
function is optimized to suffixed instance and has same name
symbol which is blacklisted.
But I would like to chose a fail-safe design for this issue.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
kernel/kprobes.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f4ddfdd2d07e..c83e54727131 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1396,7 +1396,7 @@ bool __weak arch_within_kprobe_blacklist(unsigned long addr)
addr < (unsigned long)__kprobes_text_end;
}
-bool within_kprobe_blacklist(unsigned long addr)
+static bool __within_kprobe_blacklist(unsigned long addr)
{
struct kprobe_blacklist_entry *ent;
@@ -1410,7 +1410,26 @@ bool within_kprobe_blacklist(unsigned long addr)
if (addr >= ent->start_addr && addr < ent->end_addr)
return true;
}
+ return false;
+}
+bool within_kprobe_blacklist(unsigned long addr)
+{
+ char symname[KSYM_NAME_LEN], *p;
+
+ if (__within_kprobe_blacklist(addr))
+ return true;
+
+ /* Check if the address is on a suffixed-symbol */
+ if (!lookup_symbol_name(addr, symname)) {
+ p = strchr(symname, '.');
+ if (!p)
+ return false;
+ *p = '\0';
+ addr = (unsigned long)kprobe_lookup_name(symname, 0);
+ if (addr)
+ return __within_kprobe_blacklist(addr);
+ }
return false;
}
Prohibit probing on the functions called before kprobe_int3_handler()
in do_int3(). More specifically, ftrace_int3_handler(),
poke_int3_handler(), and ist_enter(). And since rcu_nmi_enter() is
called by ist_enter(), it also should be marked as NOKPROBE_SYMBOL.
Since those are handled before kprobe_int3_handler(), probing those
functions can cause a breakpoint recursion and crash the kernel.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/x86/kernel/alternative.c | 3 ++-
arch/x86/kernel/ftrace.c | 3 ++-
arch/x86/kernel/traps.c | 1 +
kernel/rcu/tree.c | 2 ++
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index d458c7973c56..9a79c7808f9c 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -11,6 +11,7 @@
#include <linux/stop_machine.h>
#include <linux/slab.h>
#include <linux/kdebug.h>
+#include <linux/kprobes.h>
#include <asm/text-patching.h>
#include <asm/alternative.h>
#include <asm/sections.h>
@@ -764,8 +765,8 @@ int poke_int3_handler(struct pt_regs *regs)
regs->ip = (unsigned long) bp_int3_handler;
return 1;
-
}
+NOKPROBE_SYMBOL(poke_int3_handler);
/**
* text_poke_bp() -- update instructions on live kernel on SMP
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8257a59704ae..3e3789c8f8e1 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -269,7 +269,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
return ret;
}
-static int is_ftrace_caller(unsigned long ip)
+static nokprobe_inline int is_ftrace_caller(unsigned long ip)
{
if (ip == ftrace_update_func)
return 1;
@@ -299,6 +299,7 @@ int ftrace_int3_handler(struct pt_regs *regs)
return 1;
}
+NOKPROBE_SYMBOL(ftrace_int3_handler);
static int ftrace_write(unsigned long ip, const char *val, int size)
{
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 2684a9d11e66..d26f9e9c3d83 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -111,6 +111,7 @@ void ist_enter(struct pt_regs *regs)
/* This code is a bit fragile. Test it. */
RCU_LOCKDEP_WARN(!rcu_is_watching(), "ist_enter didn't work");
}
+NOKPROBE_SYMBOL(ist_enter);
void ist_exit(struct pt_regs *regs)
{
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 9180158756d2..74db52a0a466 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -62,6 +62,7 @@
#include <linux/suspend.h>
#include <linux/ftrace.h>
#include <linux/tick.h>
+#include <linux/kprobes.h>
#include "tree.h"
#include "rcu.h"
@@ -872,6 +873,7 @@ void rcu_nmi_enter(void)
{
rcu_nmi_enter_common(false);
}
+NOKPROBE_SYMBOL(rcu_nmi_enter);
/**
* rcu_irq_enter - inform RCU that current CPU is entering irq away from idle
Since kprobes breakpoint handling involves hardirq tracer,
probing these functions cause breakpoint recursion problem.
Prohibit probing on those functions.
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt (VMware) <[email protected]>
---
kernel/trace/trace_irqsoff.c | 9 +++++++--
kernel/trace/trace_preemptirq.c | 5 +++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index d3294721f119..d42a473b8240 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -14,6 +14,7 @@
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/ftrace.h>
+#include <linux/kprobes.h>
#include "trace.h"
@@ -365,7 +366,7 @@ check_critical_timing(struct trace_array *tr,
__trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
}
-static inline void
+static nokprobe_inline void
start_critical_timing(unsigned long ip, unsigned long parent_ip, int pc)
{
int cpu;
@@ -401,7 +402,7 @@ start_critical_timing(unsigned long ip, unsigned long parent_ip, int pc)
atomic_dec(&data->disabled);
}
-static inline void
+static nokprobe_inline void
stop_critical_timing(unsigned long ip, unsigned long parent_ip, int pc)
{
int cpu;
@@ -443,6 +444,7 @@ void start_critical_timings(void)
start_critical_timing(CALLER_ADDR0, CALLER_ADDR1, pc);
}
EXPORT_SYMBOL_GPL(start_critical_timings);
+NOKPROBE_SYMBOL(start_critical_timings);
void stop_critical_timings(void)
{
@@ -452,6 +454,7 @@ void stop_critical_timings(void)
stop_critical_timing(CALLER_ADDR0, CALLER_ADDR1, pc);
}
EXPORT_SYMBOL_GPL(stop_critical_timings);
+NOKPROBE_SYMBOL(stop_critical_timings);
#ifdef CONFIG_FUNCTION_TRACER
static bool function_enabled;
@@ -611,6 +614,7 @@ void tracer_hardirqs_on(unsigned long a0, unsigned long a1)
if (!preempt_trace(pc) && irq_trace())
stop_critical_timing(a0, a1, pc);
}
+NOKPROBE_SYMBOL(tracer_hardirqs_on);
void tracer_hardirqs_off(unsigned long a0, unsigned long a1)
{
@@ -619,6 +623,7 @@ void tracer_hardirqs_off(unsigned long a0, unsigned long a1)
if (!preempt_trace(pc) && irq_trace())
start_critical_timing(a0, a1, pc);
}
+NOKPROBE_SYMBOL(tracer_hardirqs_off);
static int irqsoff_tracer_init(struct trace_array *tr)
{
diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
index 71f553cceb3c..4d8e99fdbbbe 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -9,6 +9,7 @@
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/ftrace.h>
+#include <linux/kprobes.h>
#include "trace.h"
#define CREATE_TRACE_POINTS
@@ -30,6 +31,7 @@ void trace_hardirqs_on(void)
lockdep_hardirqs_on(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_on);
+NOKPROBE_SYMBOL(trace_hardirqs_on);
void trace_hardirqs_off(void)
{
@@ -43,6 +45,7 @@ void trace_hardirqs_off(void)
lockdep_hardirqs_off(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_off);
+NOKPROBE_SYMBOL(trace_hardirqs_off);
__visible void trace_hardirqs_on_caller(unsigned long caller_addr)
{
@@ -56,6 +59,7 @@ __visible void trace_hardirqs_on_caller(unsigned long caller_addr)
lockdep_hardirqs_on(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_on_caller);
+NOKPROBE_SYMBOL(trace_hardirqs_on_caller);
__visible void trace_hardirqs_off_caller(unsigned long caller_addr)
{
@@ -69,6 +73,7 @@ __visible void trace_hardirqs_off_caller(unsigned long caller_addr)
lockdep_hardirqs_off(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_off_caller);
+NOKPROBE_SYMBOL(trace_hardirqs_off_caller);
#endif /* CONFIG_TRACE_IRQFLAGS */
#ifdef CONFIG_TRACE_PREEMPT_TOGGLE
Since kprobes depends on preempt disable/enable, probing
on the preempt debug routine can cause recursive breakpoint
problem.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
lib/smp_processor_id.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c
index 85925aaa4fff..157d9e31f6c2 100644
--- a/lib/smp_processor_id.c
+++ b/lib/smp_processor_id.c
@@ -5,10 +5,11 @@
* DEBUG_PREEMPT variant of smp_processor_id().
*/
#include <linux/export.h>
+#include <linux/kprobes.h>
#include <linux/sched.h>
-notrace static unsigned int check_preemption_disabled(const char *what1,
- const char *what2)
+notrace static nokprobe_inline
+unsigned int check_preemption_disabled(const char *what1, const char *what2)
{
int this_cpu = raw_smp_processor_id();
@@ -56,9 +57,11 @@ notrace unsigned int debug_smp_processor_id(void)
return check_preemption_disabled("smp_processor_id", "");
}
EXPORT_SYMBOL(debug_smp_processor_id);
+NOKPROBE_SYMBOL(debug_smp_processor_id);
notrace void __this_cpu_preempt_check(const char *op)
{
check_preemption_disabled("__this_cpu_", op);
}
EXPORT_SYMBOL(__this_cpu_preempt_check);
+NOKPROBE_SYMBOL(__this_cpu_preempt_check);
Since kprobe itself depends on RCU, probing on RCU debug
routine can cause recursive breakpoint problem.
Prohibit probing on RCU debug routines.
int3
->do_int3()
->ist_enter()
->RCU_LOCKDEP_WARN()
->debug_lockdep_rcu_enabled() -> int3
Signed-off-by: Masami Hiramatsu <[email protected]>
---
kernel/rcu/update.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 1971869c4072..f4ca36d92138 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -52,6 +52,7 @@
#include <linux/tick.h>
#include <linux/rcupdate_wait.h>
#include <linux/sched/isolation.h>
+#include <linux/kprobes.h>
#define CREATE_TRACE_POINTS
@@ -249,6 +250,7 @@ int notrace debug_lockdep_rcu_enabled(void)
current->lockdep_recursion == 0;
}
EXPORT_SYMBOL_GPL(debug_lockdep_rcu_enabled);
+NOKPROBE_SYMBOL(debug_lockdep_rcu_enabled);
/**
* rcu_read_lock_held() - might we be in RCU read-side critical section?
Some lockdep functions can be involved in breakpoint handling
and probing on those functions can cause a breakpoint recursion.
Prohibit probing on those functions by blacklist.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
kernel/locking/lockdep.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 7f7db23fc002..c1653a1b57b7 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -50,6 +50,7 @@
#include <linux/random.h>
#include <linux/jhash.h>
#include <linux/nmi.h>
+#include <linux/kprobes.h>
#include <asm/sections.h>
@@ -2796,6 +2797,7 @@ void lockdep_hardirqs_on(unsigned long ip)
__trace_hardirqs_on_caller(ip);
current->lockdep_recursion = 0;
}
+NOKPROBE_SYMBOL(lockdep_hardirqs_on);
/*
* Hardirqs were disabled:
@@ -2825,6 +2827,7 @@ void lockdep_hardirqs_off(unsigned long ip)
} else
debug_atomic_inc(redundant_hardirqs_off);
}
+NOKPROBE_SYMBOL(lockdep_hardirqs_off);
/*
* Softirqs will be enabled:
@@ -3638,7 +3641,8 @@ __lock_release(struct lockdep_map *lock, int nested, unsigned long ip)
return 0;
}
-static int __lock_is_held(const struct lockdep_map *lock, int read)
+static nokprobe_inline
+int __lock_is_held(const struct lockdep_map *lock, int read)
{
struct task_struct *curr = current;
int i;
@@ -3871,6 +3875,7 @@ int lock_is_held_type(const struct lockdep_map *lock, int read)
return ret;
}
EXPORT_SYMBOL_GPL(lock_is_held_type);
+NOKPROBE_SYMBOL(lock_is_held_type);
struct pin_cookie lock_pin_lock(struct lockdep_map *lock)
{
From: Andrea Righi <[email protected]>
Since kprobe breakpoing handler is using bsearch(), probing on this
routine can cause recursive breakpoint problem.
int3
->do_int3()
->ftrace_int3_handler()
->ftrace_location()
->ftrace_location_range()
->bsearch() -> int3
Prohibit probing on bsearch().
Signed-off-by: Andrea Righi <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
lib/bsearch.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/bsearch.c b/lib/bsearch.c
index 18b445b010c3..82512fe7b33c 100644
--- a/lib/bsearch.c
+++ b/lib/bsearch.c
@@ -11,6 +11,7 @@
#include <linux/export.h>
#include <linux/bsearch.h>
+#include <linux/kprobes.h>
/*
* bsearch - binary search an array of elements
@@ -53,3 +54,4 @@ void *bsearch(const void *key, const void *base, size_t num, size_t size,
return NULL;
}
EXPORT_SYMBOL(bsearch);
+NOKPROBE_SYMBOL(bsearch);
* Masami Hiramatsu <[email protected]> wrote:
> Newer gcc can generate some different instances of a function
> with suffixed symbols if the function is optimized and only
> has a part of that. (e.g. .constprop, .part etc.)
>
> In this case, it is not enough to check the entry of kprobe
> blacklist because it only records non-suffixed symbol address.
>
> To fix this issue, search non-suffixed symbol in blacklist if
> given address is within a symbol which has a suffix.
>
> Note that this can cause false positive cases if a kprobe-safe
> function is optimized to suffixed instance and has same name
> symbol which is blacklisted.
> But I would like to chose a fail-safe design for this issue.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
Why did you not add Steven's Reviewed-by tag?
Thanks,
Ingo
* Ingo Molnar <[email protected]> wrote:
>
> * Masami Hiramatsu <[email protected]> wrote:
>
> > Newer gcc can generate some different instances of a function
> > with suffixed symbols if the function is optimized and only
> > has a part of that. (e.g. .constprop, .part etc.)
> >
> > In this case, it is not enough to check the entry of kprobe
> > blacklist because it only records non-suffixed symbol address.
> >
> > To fix this issue, search non-suffixed symbol in blacklist if
> > given address is within a symbol which has a suffix.
> >
> > Note that this can cause false positive cases if a kprobe-safe
> > function is optimized to suffixed instance and has same name
> > symbol which is blacklisted.
> > But I would like to chose a fail-safe design for this issue.
> >
> > Signed-off-by: Masami Hiramatsu <[email protected]>
>
> Why did you not add Steven's Reviewed-by tag?
The series looks fine otherwise, so I applied it with Steve's reviewed-by
tag added.
Thanks,
Ingo
Commit-ID: 0192e6535ebe9af68614198ced4fd6d37b778ebf
Gitweb: https://git.kernel.org/tip/0192e6535ebe9af68614198ced4fd6d37b778ebf
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:11:19 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:39 +0100
x86/kprobes: Prohibit probing on optprobe template code
Prohibit probing on optprobe template code, since it is not
a code but a template instruction sequence. If we modify
this template, copied template must be broken.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Fixes: 9326638cbee2 ("kprobes, x86: Use NOKPROBE_SYMBOL() instead of __kprobes annotation")
Link: http://lkml.kernel.org/r/154998787911.31052.15274376330136234452.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/kprobes/opt.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 6adf6e6c2933..544bd41a514c 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -141,6 +141,11 @@ asm (
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
+NOKPROBE_SYMBOL(optprobe_template_func);
+NOKPROBE_SYMBOL(optprobe_template_entry);
+NOKPROBE_SYMBOL(optprobe_template_val);
+NOKPROBE_SYMBOL(optprobe_template_call);
+NOKPROBE_SYMBOL(optprobe_template_end);
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
Commit-ID: c13324a505c7790fe91a9df35be2e0462abccdb0
Gitweb: https://git.kernel.org/tip/c13324a505c7790fe91a9df35be2e0462abccdb0
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:12:15 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:39 +0100
x86/kprobes: Prohibit probing on functions before kprobe_int3_handler()
Prohibit probing on the functions called before kprobe_int3_handler()
in do_int3(). More specifically, ftrace_int3_handler(),
poke_int3_handler(), and ist_enter(). And since rcu_nmi_enter() is
called by ist_enter(), it also should be marked as NOKPROBE_SYMBOL.
Since those are handled before kprobe_int3_handler(), probing those
functions can cause a breakpoint recursion and crash the kernel.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998793571.31052.11301258949601150994.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/alternative.c | 3 ++-
arch/x86/kernel/ftrace.c | 3 ++-
arch/x86/kernel/traps.c | 1 +
kernel/rcu/tree.c | 2 ++
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ebeac487a20c..e8b628b1b279 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -11,6 +11,7 @@
#include <linux/stop_machine.h>
#include <linux/slab.h>
#include <linux/kdebug.h>
+#include <linux/kprobes.h>
#include <asm/text-patching.h>
#include <asm/alternative.h>
#include <asm/sections.h>
@@ -764,8 +765,8 @@ int poke_int3_handler(struct pt_regs *regs)
regs->ip = (unsigned long) bp_int3_handler;
return 1;
-
}
+NOKPROBE_SYMBOL(poke_int3_handler);
/**
* text_poke_bp() -- update instructions on live kernel on SMP
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8257a59704ae..3e3789c8f8e1 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -269,7 +269,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
return ret;
}
-static int is_ftrace_caller(unsigned long ip)
+static nokprobe_inline int is_ftrace_caller(unsigned long ip)
{
if (ip == ftrace_update_func)
return 1;
@@ -299,6 +299,7 @@ int ftrace_int3_handler(struct pt_regs *regs)
return 1;
}
+NOKPROBE_SYMBOL(ftrace_int3_handler);
static int ftrace_write(unsigned long ip, const char *val, int size)
{
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 9b7c4ca8f0a7..e289ce1332ab 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -111,6 +111,7 @@ void ist_enter(struct pt_regs *regs)
/* This code is a bit fragile. Test it. */
RCU_LOCKDEP_WARN(!rcu_is_watching(), "ist_enter didn't work");
}
+NOKPROBE_SYMBOL(ist_enter);
void ist_exit(struct pt_regs *regs)
{
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 9180158756d2..74db52a0a466 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -62,6 +62,7 @@
#include <linux/suspend.h>
#include <linux/ftrace.h>
#include <linux/tick.h>
+#include <linux/kprobes.h>
#include "tree.h"
#include "rcu.h"
@@ -872,6 +873,7 @@ void rcu_nmi_enter(void)
{
rcu_nmi_enter_common(false);
}
+NOKPROBE_SYMBOL(rcu_nmi_enter);
/**
* rcu_irq_enter - inform RCU that current CPU is entering irq away from idle
Commit-ID: 877b145f0f4723133f934be402b8dfc769eb971f
Gitweb: https://git.kernel.org/tip/877b145f0f4723133f934be402b8dfc769eb971f
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:11:47 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:39 +0100
x86/kprobes: Move trampoline code into RODATA
Move optprobe trampoline code into RODATA since it is
not executed, but copied and modified to be used on
a trampoline buffer.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998790744.31052.3016106262944915510.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/kprobes/opt.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 544bd41a514c..f14262952015 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -97,6 +97,7 @@ static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
}
asm (
+ ".pushsection .rodata\n"
"optprobe_template_func:\n"
".global optprobe_template_entry\n"
"optprobe_template_entry:\n"
@@ -136,16 +137,10 @@ asm (
#endif
".global optprobe_template_end\n"
"optprobe_template_end:\n"
- ".type optprobe_template_func, @function\n"
- ".size optprobe_template_func, .-optprobe_template_func\n");
+ ".popsection\n");
void optprobe_template_func(void);
STACK_FRAME_NON_STANDARD(optprobe_template_func);
-NOKPROBE_SYMBOL(optprobe_template_func);
-NOKPROBE_SYMBOL(optprobe_template_entry);
-NOKPROBE_SYMBOL(optprobe_template_val);
-NOKPROBE_SYMBOL(optprobe_template_call);
-NOKPROBE_SYMBOL(optprobe_template_end);
#define TMPL_MOVE_IDX \
((long)optprobe_template_val - (long)optprobe_template_entry)
Commit-ID: 6143c6fb1e8f9bde9c434038f7548a19d36b55e7
Gitweb: https://git.kernel.org/tip/6143c6fb1e8f9bde9c434038f7548a19d36b55e7
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:13:12 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:40 +0100
kprobes: Search non-suffixed symbol in blacklist
Newer GCC versions can generate some different instances of a function
with suffixed symbols if the function is optimized and only
has a part of that. (e.g. .constprop, .part etc.)
In this case, it is not enough to check the entry of kprobe
blacklist because it only records non-suffixed symbol address.
To fix this issue, search non-suffixed symbol in blacklist if
given address is within a symbol which has a suffix.
Note that this can cause false positive cases if a kprobe-safe
function is optimized to suffixed instance and has same name
symbol which is blacklisted.
But I would like to chose a fail-safe design for this issue.
Signed-off-by: Masami Hiramatsu <[email protected]>
Reviewed-by: Steven Rostedt (VMware) <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998799234.31052.6136378903570418008.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/kprobes.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f4ddfdd2d07e..c83e54727131 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1396,7 +1396,7 @@ bool __weak arch_within_kprobe_blacklist(unsigned long addr)
addr < (unsigned long)__kprobes_text_end;
}
-bool within_kprobe_blacklist(unsigned long addr)
+static bool __within_kprobe_blacklist(unsigned long addr)
{
struct kprobe_blacklist_entry *ent;
@@ -1410,7 +1410,26 @@ bool within_kprobe_blacklist(unsigned long addr)
if (addr >= ent->start_addr && addr < ent->end_addr)
return true;
}
+ return false;
+}
+bool within_kprobe_blacklist(unsigned long addr)
+{
+ char symname[KSYM_NAME_LEN], *p;
+
+ if (__within_kprobe_blacklist(addr))
+ return true;
+
+ /* Check if the address is on a suffixed-symbol */
+ if (!lookup_symbol_name(addr, symname)) {
+ p = strchr(symname, '.');
+ if (!p)
+ return false;
+ *p = '\0';
+ addr = (unsigned long)kprobe_lookup_name(symname, 0);
+ if (addr)
+ return __within_kprobe_blacklist(addr);
+ }
return false;
}
Commit-ID: 984640ce427fa67c7c1f8550ab53495733bd11fc
Gitweb: https://git.kernel.org/tip/984640ce427fa67c7c1f8550ab53495733bd11fc
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:14:09 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:40 +0100
kprobes: Prohibit probing on preemption checking debug functions
Since kprobes depends on preempt disable/enable, probing
on the preempt debug routines can cause recursive breakpoint
bugs.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998804911.31052.3541963527929117920.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
lib/smp_processor_id.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c
index 85925aaa4fff..157d9e31f6c2 100644
--- a/lib/smp_processor_id.c
+++ b/lib/smp_processor_id.c
@@ -5,10 +5,11 @@
* DEBUG_PREEMPT variant of smp_processor_id().
*/
#include <linux/export.h>
+#include <linux/kprobes.h>
#include <linux/sched.h>
-notrace static unsigned int check_preemption_disabled(const char *what1,
- const char *what2)
+notrace static nokprobe_inline
+unsigned int check_preemption_disabled(const char *what1, const char *what2)
{
int this_cpu = raw_smp_processor_id();
@@ -56,9 +57,11 @@ notrace unsigned int debug_smp_processor_id(void)
return check_preemption_disabled("smp_processor_id", "");
}
EXPORT_SYMBOL(debug_smp_processor_id);
+NOKPROBE_SYMBOL(debug_smp_processor_id);
notrace void __this_cpu_preempt_check(const char *op)
{
check_preemption_disabled("__this_cpu_", op);
}
EXPORT_SYMBOL(__this_cpu_preempt_check);
+NOKPROBE_SYMBOL(__this_cpu_preempt_check);
Commit-ID: eeeb080bae906a57b6513d37efe3c38f2cb87a1c
Gitweb: https://git.kernel.org/tip/eeeb080bae906a57b6513d37efe3c38f2cb87a1c
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:13:40 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:40 +0100
kprobes: Prohibit probing on hardirq tracers
Since kprobes breakpoint handling involves hardirq tracer,
probing these functions cause breakpoint recursion problem.
Prohibit probing on those functions.
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt (VMware) <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998802073.31052.17255044712514564153.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_irqsoff.c | 9 +++++++--
kernel/trace/trace_preemptirq.c | 5 +++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index d3294721f119..d42a473b8240 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -14,6 +14,7 @@
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/ftrace.h>
+#include <linux/kprobes.h>
#include "trace.h"
@@ -365,7 +366,7 @@ out:
__trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
}
-static inline void
+static nokprobe_inline void
start_critical_timing(unsigned long ip, unsigned long parent_ip, int pc)
{
int cpu;
@@ -401,7 +402,7 @@ start_critical_timing(unsigned long ip, unsigned long parent_ip, int pc)
atomic_dec(&data->disabled);
}
-static inline void
+static nokprobe_inline void
stop_critical_timing(unsigned long ip, unsigned long parent_ip, int pc)
{
int cpu;
@@ -443,6 +444,7 @@ void start_critical_timings(void)
start_critical_timing(CALLER_ADDR0, CALLER_ADDR1, pc);
}
EXPORT_SYMBOL_GPL(start_critical_timings);
+NOKPROBE_SYMBOL(start_critical_timings);
void stop_critical_timings(void)
{
@@ -452,6 +454,7 @@ void stop_critical_timings(void)
stop_critical_timing(CALLER_ADDR0, CALLER_ADDR1, pc);
}
EXPORT_SYMBOL_GPL(stop_critical_timings);
+NOKPROBE_SYMBOL(stop_critical_timings);
#ifdef CONFIG_FUNCTION_TRACER
static bool function_enabled;
@@ -611,6 +614,7 @@ void tracer_hardirqs_on(unsigned long a0, unsigned long a1)
if (!preempt_trace(pc) && irq_trace())
stop_critical_timing(a0, a1, pc);
}
+NOKPROBE_SYMBOL(tracer_hardirqs_on);
void tracer_hardirqs_off(unsigned long a0, unsigned long a1)
{
@@ -619,6 +623,7 @@ void tracer_hardirqs_off(unsigned long a0, unsigned long a1)
if (!preempt_trace(pc) && irq_trace())
start_critical_timing(a0, a1, pc);
}
+NOKPROBE_SYMBOL(tracer_hardirqs_off);
static int irqsoff_tracer_init(struct trace_array *tr)
{
diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
index 71f553cceb3c..4d8e99fdbbbe 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -9,6 +9,7 @@
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/ftrace.h>
+#include <linux/kprobes.h>
#include "trace.h"
#define CREATE_TRACE_POINTS
@@ -30,6 +31,7 @@ void trace_hardirqs_on(void)
lockdep_hardirqs_on(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_on);
+NOKPROBE_SYMBOL(trace_hardirqs_on);
void trace_hardirqs_off(void)
{
@@ -43,6 +45,7 @@ void trace_hardirqs_off(void)
lockdep_hardirqs_off(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_off);
+NOKPROBE_SYMBOL(trace_hardirqs_off);
__visible void trace_hardirqs_on_caller(unsigned long caller_addr)
{
@@ -56,6 +59,7 @@ __visible void trace_hardirqs_on_caller(unsigned long caller_addr)
lockdep_hardirqs_on(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_on_caller);
+NOKPROBE_SYMBOL(trace_hardirqs_on_caller);
__visible void trace_hardirqs_off_caller(unsigned long caller_addr)
{
@@ -69,6 +73,7 @@ __visible void trace_hardirqs_off_caller(unsigned long caller_addr)
lockdep_hardirqs_off(CALLER_ADDR0);
}
EXPORT_SYMBOL(trace_hardirqs_off_caller);
+NOKPROBE_SYMBOL(trace_hardirqs_off_caller);
#endif /* CONFIG_TRACE_IRQFLAGS */
#ifdef CONFIG_TRACE_PREEMPT_TOGGLE
Commit-ID: 2f43c6022d84b2f562623a7023f49f1431e50747
Gitweb: https://git.kernel.org/tip/2f43c6022d84b2f562623a7023f49f1431e50747
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:15:05 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:41 +0100
kprobes: Prohibit probing on lockdep functions
Some lockdep functions can be involved in breakpoint handling
and probing on those functions can cause a breakpoint recursion.
Prohibit probing on those functions by blacklist.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998810578.31052.1680977921449292812.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/locking/lockdep.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 95932333a48b..bc35a54ae3d4 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -50,6 +50,7 @@
#include <linux/random.h>
#include <linux/jhash.h>
#include <linux/nmi.h>
+#include <linux/kprobes.h>
#include <asm/sections.h>
@@ -2814,6 +2815,7 @@ void lockdep_hardirqs_on(unsigned long ip)
__trace_hardirqs_on_caller(ip);
current->lockdep_recursion = 0;
}
+NOKPROBE_SYMBOL(lockdep_hardirqs_on);
/*
* Hardirqs were disabled:
@@ -2843,6 +2845,7 @@ void lockdep_hardirqs_off(unsigned long ip)
} else
debug_atomic_inc(redundant_hardirqs_off);
}
+NOKPROBE_SYMBOL(lockdep_hardirqs_off);
/*
* Softirqs will be enabled:
@@ -3650,7 +3653,8 @@ __lock_release(struct lockdep_map *lock, int nested, unsigned long ip)
return 0;
}
-static int __lock_is_held(const struct lockdep_map *lock, int read)
+static nokprobe_inline
+int __lock_is_held(const struct lockdep_map *lock, int read)
{
struct task_struct *curr = current;
int i;
@@ -3883,6 +3887,7 @@ int lock_is_held_type(const struct lockdep_map *lock, int read)
return ret;
}
EXPORT_SYMBOL_GPL(lock_is_held_type);
+NOKPROBE_SYMBOL(lock_is_held_type);
struct pin_cookie lock_pin_lock(struct lockdep_map *lock)
{
Commit-ID: 02106f883cd745523f7766d90a739f983f19e650
Gitweb: https://git.kernel.org/tip/02106f883cd745523f7766d90a739f983f19e650
Author: Andrea Righi <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:15:34 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:41 +0100
kprobes: Prohibit probing on bsearch()
Since kprobe breakpoing handler is using bsearch(), probing on this
routine can cause recursive breakpoint problem.
int3
->do_int3()
->ftrace_int3_handler()
->ftrace_location()
->ftrace_location_range()
->bsearch() -> int3
Prohibit probing on bsearch().
Signed-off-by: Andrea Righi <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998813406.31052.8791425358974650922.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
lib/bsearch.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/bsearch.c b/lib/bsearch.c
index 18b445b010c3..82512fe7b33c 100644
--- a/lib/bsearch.c
+++ b/lib/bsearch.c
@@ -11,6 +11,7 @@
#include <linux/export.h>
#include <linux/bsearch.h>
+#include <linux/kprobes.h>
/*
* bsearch - binary search an array of elements
@@ -53,3 +54,4 @@ void *bsearch(const void *key, const void *base, size_t num, size_t size,
return NULL;
}
EXPORT_SYMBOL(bsearch);
+NOKPROBE_SYMBOL(bsearch);
Commit-ID: a39f15b9644fac3f950f522c39e667c3af25c588
Gitweb: https://git.kernel.org/tip/a39f15b9644fac3f950f522c39e667c3af25c588
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:14:37 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:40 +0100
kprobes: Prohibit probing on RCU debug routine
Since kprobe itself depends on RCU, probing on RCU debug
routine can cause recursive breakpoint bugs.
Prohibit probing on RCU debug routines.
int3
->do_int3()
->ist_enter()
->RCU_LOCKDEP_WARN()
->debug_lockdep_rcu_enabled() -> int3
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998807741.31052.11229157537816341591.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/rcu/update.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 1971869c4072..f4ca36d92138 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -52,6 +52,7 @@
#include <linux/tick.h>
#include <linux/rcupdate_wait.h>
#include <linux/sched/isolation.h>
+#include <linux/kprobes.h>
#define CREATE_TRACE_POINTS
@@ -249,6 +250,7 @@ int notrace debug_lockdep_rcu_enabled(void)
current->lockdep_recursion == 0;
}
EXPORT_SYMBOL_GPL(debug_lockdep_rcu_enabled);
+NOKPROBE_SYMBOL(debug_lockdep_rcu_enabled);
/**
* rcu_read_lock_held() - might we be in RCU read-side critical section?
Commit-ID: 0eae81dc9f026d899c70f3931bf3bca6d7aa6938
Gitweb: https://git.kernel.org/tip/0eae81dc9f026d899c70f3931bf3bca6d7aa6938
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 13 Feb 2019 01:12:44 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 13 Feb 2019 08:16:39 +0100
x86/kprobes: Prohibit probing on IRQ handlers directly
Prohibit probing on IRQ handlers in irqentry_text because
if it interrupts user mode, at that point we haven't changed
to kernel space yet and which eventually leads a double fault.
E.g.
# echo p apic_timer_interrupt > kprobe_events
# echo 1 > events/kprobes/enable
PANIC: double fault, error_code: 0x0
CPU: 1 PID: 814 Comm: less Not tainted 4.20.0-rc3+ #30
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
RIP: 0010:error_entry+0x12/0xf0
[snip]
Call Trace:
<ENTRY_TRAMPOLINE>
? native_iret+0x7/0x7
? async_page_fault+0x8/0x30
? trace_hardirqs_on_thunk+0x1c/0x1c
? error_entry+0x7c/0xf0
? async_page_fault+0x8/0x30
? native_iret+0x7/0x7
? int3+0xa/0x20
? trace_hardirqs_on_thunk+0x1c/0x1c
? error_entry+0x7c/0xf0
? int3+0xa/0x20
? apic_timer_interrupt+0x1/0x20
</ENTRY_TRAMPOLINE>
Kernel panic - not syncing: Machine halted.
Kernel Offset: disabled
---[ end Kernel panic - not syncing: Machine halted. ]---
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrea Righi <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/154998796400.31052.8406236614820687840.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/kprobes/core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 4ba75afba527..a034cb808e7e 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1028,6 +1028,13 @@ NOKPROBE_SYMBOL(kprobe_fault_handler);
int __init arch_populate_kprobe_blacklist(void)
{
+ int ret;
+
+ ret = kprobe_add_area_blacklist((unsigned long)__irqentry_text_start,
+ (unsigned long)__irqentry_text_end);
+ if (ret)
+ return ret;
+
return kprobe_add_area_blacklist((unsigned long)__entry_text_start,
(unsigned long)__entry_text_end);
}
On Wed, 13 Feb 2019 08:17:11 +0100
Ingo Molnar <[email protected]> wrote:
> > > Signed-off-by: Masami Hiramatsu <[email protected]>
> >
> > Why did you not add Steven's Reviewed-by tag?
>
> The series looks fine otherwise, so I applied it with Steve's reviewed-by
> tag added.
Thanks Ingo!
-- Steve
On Wed, 13 Feb 2019 08:17:11 +0100
Ingo Molnar <[email protected]> wrote:
>
> * Ingo Molnar <[email protected]> wrote:
>
> >
> > * Masami Hiramatsu <[email protected]> wrote:
> >
> > > Newer gcc can generate some different instances of a function
> > > with suffixed symbols if the function is optimized and only
> > > has a part of that. (e.g. .constprop, .part etc.)
> > >
> > > In this case, it is not enough to check the entry of kprobe
> > > blacklist because it only records non-suffixed symbol address.
> > >
> > > To fix this issue, search non-suffixed symbol in blacklist if
> > > given address is within a symbol which has a suffix.
> > >
> > > Note that this can cause false positive cases if a kprobe-safe
> > > function is optimized to suffixed instance and has same name
> > > symbol which is blacklisted.
> > > But I would like to chose a fail-safe design for this issue.
> > >
> > > Signed-off-by: Masami Hiramatsu <[email protected]>
> >
> > Why did you not add Steven's Reviewed-by tag?
>
> The series looks fine otherwise, so I applied it with Steve's reviewed-by
> tag added.
Oops, sorry! I missed to add that. :-(
Thank you,
>
> Thanks,
>
> Ingo
--
Masami Hiramatsu <[email protected]>
On Wed, 13 Feb 2019 01:12:44 +0900
Masami Hiramatsu <[email protected]> wrote:
> Prohibit probing on IRQ handlers in irqentry_text because
> if it interrupts user mode, at that point we haven't changed
> to kernel space yet and which eventually leads a double fault.
> E.g.
>
> # echo p apic_timer_interrupt > kprobe_events
Hmm, this breaks one of my tests (which I probe on do_IRQ).
It's been working for years.
> # echo 1 > events/kprobes/enable
> PANIC: double fault, error_code: 0x0
> CPU: 1 PID: 814 Comm: less Not tainted 4.20.0-rc3+ #30
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> RIP: 0010:error_entry+0x12/0xf0
> [snip]
> Call Trace:
> <ENTRY_TRAMPOLINE>
> ? native_iret+0x7/0x7
> ? async_page_fault+0x8/0x30
> ? trace_hardirqs_on_thunk+0x1c/0x1c
> ? error_entry+0x7c/0xf0
> ? async_page_fault+0x8/0x30
> ? native_iret+0x7/0x7
> ? int3+0xa/0x20
> ? trace_hardirqs_on_thunk+0x1c/0x1c
> ? error_entry+0x7c/0xf0
> ? int3+0xa/0x20
> ? apic_timer_interrupt+0x1/0x20
> </ENTRY_TRAMPOLINE>
> Kernel panic - not syncing: Machine halted.
> Kernel Offset: disabled
I'm not able to reproduce this (by removing this commit).
I'm thinking something else may have changed, as I've been tracing
interrupt entries for years, and interrupting userspace while doing
this.
I've even added probes where ftrace isn't (where it uses an int3) and
still haven't hit a problem.
I think this patch is swatting a symptom of a bug and not addressing
the bug itself. Can you send me the config that triggers this?
-- Steve
> ---[ end Kernel panic - not syncing: Machine halted. ]---
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> arch/x86/kernel/kprobes/core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 4ba75afba527..a034cb808e7e 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -1028,6 +1028,13 @@ NOKPROBE_SYMBOL(kprobe_fault_handler);
>
> int __init arch_populate_kprobe_blacklist(void)
> {
> + int ret;
> +
> + ret = kprobe_add_area_blacklist((unsigned long)__irqentry_text_start,
> + (unsigned long)__irqentry_text_end);
> + if (ret)
> + return ret;
> +
> return kprobe_add_area_blacklist((unsigned long)__entry_text_start,
> (unsigned long)__entry_text_end);
> }
On Mon, 25 Mar 2019 17:23:34 -0400
Steven Rostedt <[email protected]> wrote:
> On Wed, 13 Feb 2019 01:12:44 +0900
> Masami Hiramatsu <[email protected]> wrote:
>
> > Prohibit probing on IRQ handlers in irqentry_text because
> > if it interrupts user mode, at that point we haven't changed
> > to kernel space yet and which eventually leads a double fault.
> > E.g.
> >
> > # echo p apic_timer_interrupt > kprobe_events
>
> Hmm, this breaks one of my tests (which I probe on do_IRQ).
OK, it seems this patch is a bit redundant, because
I found that these interrupt handler issue has been fixed
by Andrea's commit before merge this patch.
commit a50480cb6d61d5c5fc13308479407b628b6bc1c5
Author: Andrea Righi <[email protected]>
Date: Thu Dec 6 10:56:48 2018 +0100
kprobes/x86: Blacklist non-attachable interrupt functions
These interrupt functions are already non-attachable by kprobes.
Blacklist them explicitly so that they can show up in
/sys/kernel/debug/kprobes/blacklist and tools like BCC can use this
additional information.
This description is a bit odd (maybe his patch is after mine?) I think
while updating this series, the patches were merged out of order.
Anyway, with above patch, the core problematic probe points are blacklisted.
>
> It's been working for years.
>
>
> > # echo 1 > events/kprobes/enable
> > PANIC: double fault, error_code: 0x0
> > CPU: 1 PID: 814 Comm: less Not tainted 4.20.0-rc3+ #30
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> > RIP: 0010:error_entry+0x12/0xf0
> > [snip]
> > Call Trace:
> > <ENTRY_TRAMPOLINE>
> > ? native_iret+0x7/0x7
> > ? async_page_fault+0x8/0x30
> > ? trace_hardirqs_on_thunk+0x1c/0x1c
> > ? error_entry+0x7c/0xf0
> > ? async_page_fault+0x8/0x30
> > ? native_iret+0x7/0x7
> > ? int3+0xa/0x20
> > ? trace_hardirqs_on_thunk+0x1c/0x1c
> > ? error_entry+0x7c/0xf0
> > ? int3+0xa/0x20
> > ? apic_timer_interrupt+0x1/0x20
> > </ENTRY_TRAMPOLINE>
> > Kernel panic - not syncing: Machine halted.
> > Kernel Offset: disabled
>
> I'm not able to reproduce this (by removing this commit).
I ensured that if I revert both of this patch and Andrea's patch,
I can reproduce this with probing on apic_timer_interrupt().
> I'm thinking something else may have changed, as I've been tracing
> interrupt entries for years, and interrupting userspace while doing
> this.
>
> I've even added probes where ftrace isn't (where it uses an int3) and
> still haven't hit a problem.
>
> I think this patch is swatting a symptom of a bug and not addressing
> the bug itself. Can you send me the config that triggers this?
Yes, it seems you're right. Andrea's commit specifically fixed the
issue and mine is redundant. (I'm not sure why do_IRQ is in
__irqentry_text...)
So, Ingo, please revert this, since this bug already has been fixed by
commit a50480cb6d61 ("kprobes: x86_64: blacklist non-attachable interrupt
functions")
BTW, for further error investigation, I attached my kconfig which is
usually I'm testing (some options can be changed) on Qemu.
I'm using my mini-container shellscript ( https://github.com/mhiramat/mincs
) which supports qemu-container.
Thank you,
--
Masami Hiramatsu <[email protected]>
On Tue, Mar 26, 2019 at 11:50:52PM +0900, Masami Hiramatsu wrote:
> On Mon, 25 Mar 2019 17:23:34 -0400
> Steven Rostedt <[email protected]> wrote:
>
> > On Wed, 13 Feb 2019 01:12:44 +0900
> > Masami Hiramatsu <[email protected]> wrote:
> >
> > > Prohibit probing on IRQ handlers in irqentry_text because
> > > if it interrupts user mode, at that point we haven't changed
> > > to kernel space yet and which eventually leads a double fault.
> > > E.g.
> > >
> > > # echo p apic_timer_interrupt > kprobe_events
> >
> > Hmm, this breaks one of my tests (which I probe on do_IRQ).
>
> OK, it seems this patch is a bit redundant, because
> I found that these interrupt handler issue has been fixed
> by Andrea's commit before merge this patch.
>
> commit a50480cb6d61d5c5fc13308479407b628b6bc1c5
> Author: Andrea Righi <[email protected]>
> Date: Thu Dec 6 10:56:48 2018 +0100
>
> kprobes/x86: Blacklist non-attachable interrupt functions
>
> These interrupt functions are already non-attachable by kprobes.
> Blacklist them explicitly so that they can show up in
> /sys/kernel/debug/kprobes/blacklist and tools like BCC can use this
> additional information.
>
> This description is a bit odd (maybe his patch is after mine?) I think
> while updating this series, the patches were merged out of order.
> Anyway, with above patch, the core problematic probe points are blacklisted.
This is the previous thread when I posted my patch (not sure if it helps
to figure out what happened - maybe it was just an out of order merge
issue, like you said):
https://lkml.org/lkml/2018/12/6/212
>
> >
> > It's been working for years.
> >
> >
> > > # echo 1 > events/kprobes/enable
> > > PANIC: double fault, error_code: 0x0
> > > CPU: 1 PID: 814 Comm: less Not tainted 4.20.0-rc3+ #30
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> > > RIP: 0010:error_entry+0x12/0xf0
> > > [snip]
> > > Call Trace:
> > > <ENTRY_TRAMPOLINE>
> > > ? native_iret+0x7/0x7
> > > ? async_page_fault+0x8/0x30
> > > ? trace_hardirqs_on_thunk+0x1c/0x1c
> > > ? error_entry+0x7c/0xf0
> > > ? async_page_fault+0x8/0x30
> > > ? native_iret+0x7/0x7
> > > ? int3+0xa/0x20
> > > ? trace_hardirqs_on_thunk+0x1c/0x1c
> > > ? error_entry+0x7c/0xf0
> > > ? int3+0xa/0x20
> > > ? apic_timer_interrupt+0x1/0x20
> > > </ENTRY_TRAMPOLINE>
> > > Kernel panic - not syncing: Machine halted.
> > > Kernel Offset: disabled
> >
> > I'm not able to reproduce this (by removing this commit).
>
> I ensured that if I revert both of this patch and Andrea's patch,
> I can reproduce this with probing on apic_timer_interrupt().
>
> > I'm thinking something else may have changed, as I've been tracing
> > interrupt entries for years, and interrupting userspace while doing
> > this.
> >
> > I've even added probes where ftrace isn't (where it uses an int3) and
> > still haven't hit a problem.
> >
> > I think this patch is swatting a symptom of a bug and not addressing
> > the bug itself. Can you send me the config that triggers this?
>
> Yes, it seems you're right. Andrea's commit specifically fixed the
> issue and mine is redundant. (I'm not sure why do_IRQ is in
> __irqentry_text...)
Not sure if there are specific reasons for that, but do_IRQ is part of
__irqentry_text because it's explicitly marked with __irq_entry.
>
> So, Ingo, please revert this, since this bug already has been fixed by
> commit a50480cb6d61 ("kprobes: x86_64: blacklist non-attachable interrupt
> functions")
>
> BTW, for further error investigation, I attached my kconfig which is
> usually I'm testing (some options can be changed) on Qemu.
> I'm using my mini-container shellscript ( https://github.com/mhiramat/mincs
> ) which supports qemu-container.
>
>
> Thank you,
>
> --
> Masami Hiramatsu <[email protected]>
Thanks,
-Andrea