[PATCH -tip 0/2] kprobes: fix bugs by updating blacklist

[PATCH -tip 0/2] kprobes: fix bugs by updating blacklist

[Masami Hiramatsu]

Recently I've found that putting probes on some functions
could lock or reboot the kernel. These two patches fix
those bugs by prohibiting probing such functions by
adding those to blacklist(kprobes.text).

In both cases, those functions are related to int3 handling
execution path. One is related to irqoff-tracer and the other
is related to notifier debug option.

Thank you,

---

Masami Hiramatsu (2):
      [BUGFIX] kprobes/x86: Prohibit probing on debug_stack_*
      [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text


 arch/x86/kernel/cpu/common.c |    7 ++++---
 kernel/extable.c             |    3 ++-
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH -tip 1/2] [BUGFIX] kprobes/x86: Prohibit probing on debug_stack_*

[Masami Hiramatsu]

Prohibit probing on debug_stack_reset and debug_stack_set_zero.
Since the both functions are called from TRACE_IRQS_ON/OFF_DEBUG
macros which run in int3 ist entry, probing it may cause a soft
lockup.

This happens when the kernel built with CONFIG_DYNAMIC_FTRACE=y
and CONFIG_TRACE_IRQFLAGS=y.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Seiji Aguchi <seiji.aguchi@hds.com>
---
 arch/x86/kernel/cpu/common.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 35e28b0..8712a1a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -11,6 +11,7 @@
 #include <linux/kgdb.h>
 #include <linux/smp.h>
 #include <linux/io.h>
+#include <linux/kprobes.h>
 
 #include <asm/stackprotector.h>
 #include <asm/perf_event.h>
@@ -1158,7 +1159,7 @@ DEFINE_PER_CPU(struct orig_ist, orig_ist);
 static DEFINE_PER_CPU(unsigned long, debug_stack_addr);
 DEFINE_PER_CPU(int, debug_stack_usage);
 
-int is_debug_stack(unsigned long addr)
+int __kprobes is_debug_stack(unsigned long addr)
 {
 	return __get_cpu_var(debug_stack_usage) ||
 		(addr <= __get_cpu_var(debug_stack_addr) &&
@@ -1167,13 +1168,13 @@ int is_debug_stack(unsigned long addr)
 
 DEFINE_PER_CPU(u32, debug_idt_ctr);
 
-void debug_stack_set_zero(void)
+void __kprobes debug_stack_set_zero(void)
 {
 	this_cpu_inc(debug_idt_ctr);
 	load_current_idt();
 }
 
-void debug_stack_reset(void)
+void __kprobes debug_stack_reset(void)
 {
 	if (WARN_ON(!this_cpu_read(debug_idt_ctr)))
 		return;

Re: [PATCH -tip 1/2] [BUGFIX] kprobes/x86: Prohibit probing on debug_stack_*

[Ingo Molnar]

* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> -int is_debug_stack(unsigned long addr)
> +int __kprobes is_debug_stack(unsigned long addr)

_Please_ add a __noprobes method, for new annotations.

Naming it '__kprobes' is actively confusing, as it suggests that the 
function is somehow positively involved with kprobes support. 
Instead it should the noinline/notrace pattern.

I complained about this before and not much happened on this front. 
We might not want to convert the whole kernel straight away, but for 
_new_ annotations there's no excuse not to name them properly. Lets 
phase out __kprobes, and use __noprobe from now on, okay?

Thanks,

	Ingo

Re: [PATCH -tip 1/2] [BUGFIX] kprobes/x86: Prohibit probing on debug_stack_*

[Masami Hiramatsu]

(2013/10/31 17:22), Ingo Molnar wrote:
> 
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> -int is_debug_stack(unsigned long addr)
>> +int __kprobes is_debug_stack(unsigned long addr)
> 
> _Please_ add a __noprobes method, for new annotations.

Ah, OK. I'll send new series.

> Naming it '__kprobes' is actively confusing, as it suggests that the 
> function is somehow positively involved with kprobes support. 
> Instead it should the noinline/notrace pattern.
> 
> I complained about this before and not much happened on this front. 
> We might not want to convert the whole kernel straight away, but for 
> _new_ annotations there's no excuse not to name them properly. Lets 
> phase out __kprobes, and use __noprobe from now on, okay?

I see it. :)

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH -tip 2/2] [BUGFIX] kprobes: Prohibit probing on func_ptr_is_kernel_text

[Masami Hiramatsu]

Prohibit probing on func_ptr_is_kernel_text().
Since the func_ptr_is_kernel_text() is called from
notifier_call_chain() which is called from int3 handler,
probing it may cause double int3 fault and kernel will
reboot.

This happenes when the kernel built with CONFIG_DEBUG_NOTIFIERS=y.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Ingo Molnar <mingo@kernel.org>
---
 kernel/extable.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/extable.c b/kernel/extable.c
index 832cb28..b3b8b6a 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/init.h>
+#include <linux/kprobes.h>
 
 #include <asm/sections.h>
 #include <asm/uaccess.h>
@@ -129,7 +130,7 @@ int kernel_text_address(unsigned long addr)
  * pointer is part of the kernel text, we need to do some
  * special dereferencing first.
  */
-int func_ptr_is_kernel_text(void *ptr)
+int __kprobes func_ptr_is_kernel_text(void *ptr)
 {
 	unsigned long addr;
 	addr = (unsigned long) dereference_function_descriptor(ptr);