Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1687500 > unrolled thread

[PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist

Started byFrancis Deslauriers <francis.deslauriers@efficios.com>
First post2017-07-14 17:10 +0200
Last post2017-07-17 20:50 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist Francis Deslauriers <francis.deslauriers@efficios.com> - 2017-07-14 17:10 +0200
    Re: [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist Steven Rostedt <rostedt@goodmis.org> - 2017-07-14 20:30 +0200
      Re: [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist Masami Hiramatsu <mhiramat@kernel.org> - 2017-07-16 18:00 +0200
    Re: [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist Masami Hiramatsu <mhiramat@kernel.org> - 2017-07-16 16:40 +0200
      Re: [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist Masami Hiramatsu <mhiramat@kernel.org> - 2017-07-16 17:50 +0200
        Re: [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist Francis Deslauriers <francis.deslauriers@efficios.com> - 2017-07-17 20:50 +0200

#1687500 — [PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist

FromFrancis Deslauriers <francis.deslauriers@efficios.com>
Date2017-07-14 17:10 +0200
Subject[PATCH 0/2] kprobe: Fix: add symbols to kprobe blacklist
Message-ID<u3ao9-58i-9@gated-at.bofh.it>
Hi all,

While fuzzing the Perf kprobe and kretprobe interfaces, I found some inputs
that trigger crashes of a 4.12 kernel(6f7da290413ba713f0cdd9ff1a2a9bb129ef4f6c)
on a x86-64 VM. I know that K(ret)probes can crash the kernel in multiple ways
but should Perf be allowed to do it?

To do this analysis, I used the symbols reported by /proc/kallsyms in
conjonction with the Perf debugfs interface. Using this technique, I was able
to find two instrumentation configurations that could crash the kernel. I am
suggesting changes that fixed both issues for me by blacklisting the symbols in
question.

Kprobe on apic_timer_interrupt:
I believe that this is caused by the fact that kprobe adds a INT3 in a apic
interrupt routine.
How to reproduce:
	echo 'p:event1 apic_timer_interrupt ' > kprobe_events
	<Generate kernel activity. e.g. launch bash>
Crash log:[1]

This can be fixed by blacklisting the apicinterrupt3 symbols directly in the
assembly macro. See patch[1/2]. I am not sure that blacklisting all
apicinterrupt symbols is the right solution.


Kretprobe on ftrace_ops_assist_func and another function:
Those crashes are triggered when hooking a kretprobe on the
ftrace_ops_assist_func symbol and some other functions to make the this first
function reacheable. From my understanding, ftrace_ops_assist_func is the
function called directly when the kprobe is hit. Thus it should be marked
with NOKPROBE_SYMBOL.

Here are some configurations that can easily reproduce this bug. Those other
functions are called during the fork of a process so they are easy to control.
Enable the following kprobes and launch a process to trigger a fork to see the
kernel crash.

Conf #1
	echo 'r:event1 ftrace_ops_assist_func' > kprobe_events
	echo 'r:event2 clear_all_latency_tracing' > kprobe_events
Crash log:[2]

Conf #2
	echo 'r:event1 ftrace_ops_assist_func' > kprobe_events
	echo 'r:event2 acct_clear_integrals' > kprobe_events
Crash log:[3]

Conf #3
	echo 'r:event1 ftrace_ops_assist_func' > kprobe_events
	echo 'r:event2 arch_dup_task_struct' > kprobe_events
Crash log:[4]

The ftrace_ops_assist_func should be included in the kprobe blacklist using
NOKPROBE_SYMBOL. See patch [2/2].

Since those were found using fuzzing, it's not an exhaustive analysis.
Here is the .config I am using[5].

Thanks,

Francis Deslauriers
EfficiOS inc.


[1]: https://pastebin.com/Mpp9Yzqb
[2]: https://pastebin.com/CtsfzUwG
[3]: https://pastebin.com/txxuJXrz
[4]: https://pastebin.com/8qrJvzD3
[5]: https://pastebin.com/x5q0sgyK

Francis Deslauriers (2):
  kprobe: fix: Add _ASM_NOKPROBE to x86 apic interrupt macro
  kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

 arch/x86/entry/entry_64.S | 1 +
 kernel/trace/ftrace.c     | 2 ++
 2 files changed, 3 insertions(+)

-- 
2.7.4

[toc] | [next] | [standalone]


#1687599

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-07-14 20:30 +0200
Message-ID<u3dvH-7f8-13@gated-at.bofh.it>
In reply to#1687500
On Fri, 14 Jul 2017 10:58:33 -0400
Francis Deslauriers <francis.deslauriers@efficios.com> wrote:


> Kretprobe on ftrace_ops_assist_func and another function:
> Those crashes are triggered when hooking a kretprobe on the
> ftrace_ops_assist_func symbol and some other functions to make the this first
> function reacheable. From my understanding, ftrace_ops_assist_func is the
> function called directly when the kprobe is hit. Thus it should be marked
> with NOKPROBE_SYMBOL.
> 

Hmm, I'm wondering if I should just make an ftrace section, and black
list the entire thing. Also that section could be used to not allow
ftrace to use it either. I've been wanting to start letting ftrace
trace the tracing code, and perf for that matter. It would be nice to
be able to debug things like that.

I would like to also make sections that can be enabled or disabled in
groups. To group things like the tracing facility and perf and have
them by default not be traced, but then set a flag that says "sure go
ahead and trace them". This shouldn't be too hard to do.

Hmm, I'll add this as another topic to have for the Linux Plumbers
tracing track, as well as the kernel tracing topic.

-- Steve

[toc] | [prev] | [next] | [standalone]


#1688500

FromMasami Hiramatsu <mhiramat@kernel.org>
Date2017-07-16 18:00 +0200
Message-ID<u3U7D-Ze-1@gated-at.bofh.it>
In reply to#1687599
On Fri, 14 Jul 2017 14:27:56 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 14 Jul 2017 10:58:33 -0400
> Francis Deslauriers <francis.deslauriers@efficios.com> wrote:
> 
> 
> > Kretprobe on ftrace_ops_assist_func and another function:
> > Those crashes are triggered when hooking a kretprobe on the
> > ftrace_ops_assist_func symbol and some other functions to make the this first
> > function reacheable. From my understanding, ftrace_ops_assist_func is the
> > function called directly when the kprobe is hit. Thus it should be marked
> > with NOKPROBE_SYMBOL.
> > 
> 
> Hmm, I'm wondering if I should just make an ftrace section, and black
> list the entire thing. Also that section could be used to not allow
> ftrace to use it either. I've been wanting to start letting ftrace
> trace the tracing code, and perf for that matter. It would be nice to
> be able to debug things like that.

Tracer usually has 2 parts, one is off-line setting part (kicked by
user) and another is core online tracing part (which kicked from
anywhere). Former can be traced but latter is not.
Yeah, I did same thing when I introduced NOKPROBE_SYMBOL() macro.
And I also think that is good for kgdb.

> I would like to also make sections that can be enabled or disabled in
> groups. To group things like the tracing facility and perf and have
> them by default not be traced, but then set a flag that says "sure go
> ahead and trace them". This shouldn't be too hard to do.

We have to notice that is a trigger which allows to shoot yourself
in the foot :)

Thanks,

> 
> Hmm, I'll add this as another topic to have for the Linux Plumbers
> tracing track, as well as the kernel tracing topic.
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1688381

FromMasami Hiramatsu <mhiramat@kernel.org>
Date2017-07-16 16:40 +0200
Message-ID<u3SSi-fg-79@gated-at.bofh.it>
In reply to#1687500
Hi Francis,

At first, thank you for reporting the report!

On Fri, 14 Jul 2017 10:58:33 -0400
Francis Deslauriers <francis.deslauriers@efficios.com> wrote:

> Hi all,
> 
> While fuzzing the Perf kprobe and kretprobe interfaces, I found some inputs
> that trigger crashes of a 4.12 kernel(6f7da290413ba713f0cdd9ff1a2a9bb129ef4f6c)
> on a x86-64 VM. I know that K(ret)probes can crash the kernel in multiple ways
> but should Perf be allowed to do it?

No, it must not be, it should be fixed.

> To do this analysis, I used the symbols reported by /proc/kallsyms in
> conjonction with the Perf debugfs interface. Using this technique, I was able
> to find two instrumentation configurations that could crash the kernel. I am
> suggesting changes that fixed both issues for me by blacklisting the symbols in
> question.
> 
> Kprobe on apic_timer_interrupt:
> I believe that this is caused by the fact that kprobe adds a INT3 in a apic
> interrupt routine.
> How to reproduce:
> 	echo 'p:event1 apic_timer_interrupt ' > kprobe_events
> 	<Generate kernel activity. e.g. launch bash>
> Crash log:[1]

OK, I got this was reproduced even on qemu. (and you also seems to use qemu)

> This can be fixed by blacklisting the apicinterrupt3 symbols directly in the
> assembly macro. See patch[1/2]. I am not sure that blacklisting all
> apicinterrupt symbols is the right solution.

Wait, would you find the root cause of this crash? Your log [1] doesn't
show why this happens. It just shows there was an infinit call/fault
loop and kernel stack overflow. We should carefully analyze how it 
happened before just blacklisting it.
(BTW, ahead to keep notice, but this problem can be solved by disabling
optprobe via /proc/sys/debug/kprobes-optimization.)

[  161.618249]  ? trace_hardirqs_off_caller+0x7/0xa0
[  161.618965]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[  161.619677]  ? native_iret+0x7/0x7
[  161.620282]  ? error_entry+0x72/0xd0
[  161.620901]  ? error_entry+0x72/0xd0
[  161.621517]  ? async_page_fault+0x12/0x30
[  161.622207]  ? native_iret+0x7/0x7
[  161.622819]  ? error_entry+0x72/0xd0
[  161.623435]  ? trace_hardirqs_off_caller+0x7/0xa0
[  161.624176]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[  161.624893]  ? native_iret+0x7/0x7
[  161.625496]  ? error_entry+0x72/0xd0
[  161.626115]  ? async_page_fault+0x12/0x30
[  161.626771]  ? optimized_callback+0x17/0xf0
[  161.627443]  ? 0xffffffffa000202f

$ eu-addr2line -e vmlinux optimized_callback+0x17
/home/mhiramat/ksrc/linux/include/linux/kprobes.h:384
----
    382 static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
    383 {
    384         return this_cpu_ptr(&kprobe_ctlblk);
    385 }

And the disasm is;
ffffffff8103d716:       53                      push   %rbx
ffffffff8103d717:       65 4c 03 25 11 ca fc    add    %gs:0x7efcca11(%rip),%r12        # a130 <this_cpu_off>
ffffffff8103d71e:       7e 
----

OK, it seems this_cpu_ptr() caused pagefault.

And it seems very odd stack, since we have a few unique addresses on there.

(1) async_page_fault+0x12
----
ffffffff815f6480 <async_page_fault>:
[...]
ffffffff815f6489:       48 83 c4 88             add    $0xffffffffffffff88,%rsp
ffffffff815f648d:       e8 2e 01 00 00          callq  ffffffff815f65c0 <error_entry>
ffffffff815f6492:       48 89 e7                mov    %rsp,%rdi
----

(2) error_entry+0x72
----
ffffffff815f65c0 <error_entry>:
[...]
ffffffff815f662d:       e8 88 ab a0 ff          callq  ffffffff810011ba <trace_hardirqs_off_thunk>
ffffffff815f6632:       c3                      retq   
----

(3) trace_hardirqs_off_thunk+0x1a
----
ffffffff810011ba <trace_hardirqs_off_thunk>:
[...]
ffffffff810011cf:       e8 3c 2f 0a 00          callq  ffffffff810a4110 <trace_hardirqs_off_caller>
ffffffff810011d4:       eb 18                   jmp    ffffffff810011ee <lockdep_sys_exit_thunk+0x18>
----

(4) trace_hardirqs_off_caller+0x7
----
ffffffff810a4110:       44 8b 05 b9 11 a1 00    mov    0xa111b9(%rip),%r8d        # ffffffff81ab52d0 <debug_locks>
ffffffff810a4117:       65 48 8b 14 25 00 c4    mov    %gs:0xc400,%rdx
ffffffff810a411e:       00 00 
----

(5) native_iret+0x7 -> this seems native_irq_return_iret
----
ffffffff815f5d00 <native_iret>:
ffffffff815f5d00:       f6 44 24 20 04          testb  $0x4,0x20(%rsp)
ffffffff815f5d05:       75 02                   jne    ffffffff815f5d09 <native_irq_return_ldt>

ffffffff815f5d07 <native_irq_return_iret>:
ffffffff815f5d07:       48 cf                   iretq  
----

So, the story what the stack said,

- optimized_callback() calls get_kprobe_ctlblk(), and this_cpu_ptr(&kprobe_ctlblk) caused a page fault (in apic_timer_interrupt, does it cause any problem?)
- and following call-chain occured
  async_page_fault -> error_entry -> trace_hardirqs_off_thunk -> 
  trace_hardirqs_off_caller
- "mov    %gs:0xc400,%rdx" caused async_page_fault() again.

Since trace_hardirqs_off_thunk() stores general registers on
the stack, there are some noises.

[  114.429637] FS:  00000000021e7880(0000) GS:ffff88001fd40000(0000) knlGS:0000000000000000

So, the problem seems that cpu can not access to per-cpu pages.


> Kretprobe on ftrace_ops_assist_func and another function:

For this problem, Steve gave us an good idea so I agree with him.

Thank you,

> Those crashes are triggered when hooking a kretprobe on the
> ftrace_ops_assist_func symbol and some other functions to make the this first
> function reacheable. From my understanding, ftrace_ops_assist_func is the
> function called directly when the kprobe is hit. Thus it should be marked
> with NOKPROBE_SYMBOL.
> 
> Here are some configurations that can easily reproduce this bug. Those other
> functions are called during the fork of a process so they are easy to control.
> Enable the following kprobes and launch a process to trigger a fork to see the
> kernel crash.
> 
> Conf #1
> 	echo 'r:event1 ftrace_ops_assist_func' > kprobe_events
> 	echo 'r:event2 clear_all_latency_tracing' > kprobe_events
> Crash log:[2]
> 
> Conf #2
> 	echo 'r:event1 ftrace_ops_assist_func' > kprobe_events
> 	echo 'r:event2 acct_clear_integrals' > kprobe_events
> Crash log:[3]
> 
> Conf #3
> 	echo 'r:event1 ftrace_ops_assist_func' > kprobe_events
> 	echo 'r:event2 arch_dup_task_struct' > kprobe_events
> Crash log:[4]
> 
> The ftrace_ops_assist_func should be included in the kprobe blacklist using
> NOKPROBE_SYMBOL. See patch [2/2].
> 
> Since those were found using fuzzing, it's not an exhaustive analysis.
> Here is the .config I am using[5].
> 
> Thanks,
> 
> Francis Deslauriers
> EfficiOS inc.
> 
> 
> [1]: https://pastebin.com/Mpp9Yzqb
> [2]: https://pastebin.com/CtsfzUwG
> [3]: https://pastebin.com/txxuJXrz
> [4]: https://pastebin.com/8qrJvzD3
> [5]: https://pastebin.com/x5q0sgyK
> 
> Francis Deslauriers (2):
>   kprobe: fix: Add _ASM_NOKPROBE to x86 apic interrupt macro
>   kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist
> 
>  arch/x86/entry/entry_64.S | 1 +
>  kernel/trace/ftrace.c     | 2 ++
>  2 files changed, 3 insertions(+)
> 
> -- 
> 2.7.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1688497

FromMasami Hiramatsu <mhiramat@kernel.org>
Date2017-07-16 17:50 +0200
Message-ID<u3TXX-W2-3@gated-at.bofh.it>
In reply to#1688381
On Sun, 16 Jul 2017 23:37:44 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> So, the story what the stack said,
> 
> - optimized_callback() calls get_kprobe_ctlblk(), and this_cpu_ptr(&kprobe_ctlblk) caused a page fault (in apic_timer_interrupt, does it cause any problem?)
> - and following call-chain occured
>   async_page_fault -> error_entry -> trace_hardirqs_off_thunk -> 
>   trace_hardirqs_off_caller
> - "mov    %gs:0xc400,%rdx" caused async_page_fault() again.
> 
> Since trace_hardirqs_off_thunk() stores general registers on
> the stack, there are some noises.
> 
> [  114.429637] FS:  00000000021e7880(0000) GS:ffff88001fd40000(0000) knlGS:0000000000000000
> 
> So, the problem seems that cpu can not access to per-cpu pages.

OK, I got the root cause of this issue. Since at the irqentry code,
segment registers are not prepared for kernel yet (e.g. interrupted
in user-mode), so we must not optimize it.
I found we had already checked that by checking __entry_text_start/end,
but that is not enough, we need irqentry_text check too.

Here I made another patch, please try it.

-----
kprobes/x86: Do not jump-optimize kprobes on irq entry code

From: Masami Hiramatsu <mhiramat@kernel.org>

Since the segment registers are not prepared for kernel
in the irq-entry code, if a kprobe on such code is
jump-optimized, accessing per-cpu variables may cause
kernel panic.
However, if the kprobe is not optimized, it kicks int3
exception and set segment registers correctly.

This checks probe-address and if it is in irq-entry code,
it prohibits optimizing such kprobes. This means we can
continuously probing such interrupt handlers by kprobes
but it is not optimized anymore.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 arch/x86/entry/entry_64.S      |    2 +-
 arch/x86/include/asm/unwind.h  |    1 +
 arch/x86/kernel/kprobes/opt.c  |    4 ++--
 arch/x86/kernel/unwind_frame.c |    4 ++--
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index a9a8027..95bca8b 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -675,7 +675,7 @@ apicinterrupt3 \num trace(\sym) smp_trace(\sym)
 #endif
 
 /* Make sure APIC interrupt handlers end up in the irqentry section: */
-#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || defined(CONFIG_KPROBES)
 # define PUSH_SECTION_IRQENTRY	.pushsection .irqentry.text, "ax"
 # define POP_SECTION_IRQENTRY	.popsection
 #else
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index e667649..a9896fb9 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -28,6 +28,7 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task,
 bool unwind_next_frame(struct unwind_state *state);
 
 unsigned long unwind_get_return_address(struct unwind_state *state);
+bool in_entry_code(unsigned long ip);
 
 static inline bool unwind_done(struct unwind_state *state)
 {
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 69ea0bc..a51c144 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -39,6 +39,7 @@
 #include <asm/insn.h>
 #include <asm/debugreg.h>
 #include <asm/set_memory.h>
+#include <asm/unwind.h>
 
 #include "common.h"
 
@@ -253,8 +254,7 @@ static int can_optimize(unsigned long paddr)
 	 * Do not optimize in the entry code due to the unstable
 	 * stack handling.
 	 */
-	if ((paddr >= (unsigned long)__entry_text_start) &&
-	    (paddr <  (unsigned long)__entry_text_end))
+	if (in_entry_code(paddr))
 		return 0;
 
 	/* Check there is enough space for a relative jump. */
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index b9389d7..95123ce 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -84,14 +84,14 @@ static size_t regs_size(struct pt_regs *regs)
 	return sizeof(*regs);
 }
 
-static bool in_entry_code(unsigned long ip)
+bool in_entry_code(unsigned long ip)
 {
 	char *addr = (char *)ip;
 
 	if (addr >= __entry_text_start && addr < __entry_text_end)
 		return true;
 
-#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || defined(CONFIG_KPROBES)
 	if (addr >= __irqentry_text_start && addr < __irqentry_text_end)
 		return true;
 #endif
-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1689345

FromFrancis Deslauriers <francis.deslauriers@efficios.com>
Date2017-07-17 20:50 +0200
Message-ID<u4jfH-dY-1@gated-at.bofh.it>
In reply to#1688497
2017-07-16 11:46 GMT-04:00 Masami Hiramatsu <mhiramat@kernel.org>:
>
> On Sun, 16 Jul 2017 23:37:44 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > So, the story what the stack said,
> >
> > - optimized_callback() calls get_kprobe_ctlblk(), and this_cpu_ptr(&kprobe_ctlblk) caused a page fault (in apic_timer_interrupt, does it cause any problem?)
> > - and following call-chain occured
> >   async_page_fault -> error_entry -> trace_hardirqs_off_thunk ->
> >   trace_hardirqs_off_caller
> > - "mov    %gs:0xc400,%rdx" caused async_page_fault() again.
> >
> > Since trace_hardirqs_off_thunk() stores general registers on
> > the stack, there are some noises.
> >
> > [  114.429637] FS:  00000000021e7880(0000) GS:ffff88001fd40000(0000) knlGS:0000000000000000
> >
> > So, the problem seems that cpu can not access to per-cpu pages.
>
> OK, I got the root cause of this issue. Since at the irqentry code,
> segment registers are not prepared for kernel yet (e.g. interrupted
> in user-mode), so we must not optimize it.
> I found we had already checked that by checking __entry_text_start/end,
> but that is not enough, we need irqentry_text check too.
>
> Here I made another patch, please try it.

Hi,

This patch fixes the apic_timer_interrupt crash I was seeing. Thank you!
Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>

>
> -----
> kprobes/x86: Do not jump-optimize kprobes on irq entry code
>
> From: Masami Hiramatsu <mhiramat@kernel.org>
>
> Since the segment registers are not prepared for kernel
> in the irq-entry code, if a kprobe on such code is
> jump-optimized, accessing per-cpu variables may cause
> kernel panic.
> However, if the kprobe is not optimized, it kicks int3
> exception and set segment registers correctly.
>
> This checks probe-address and if it is in irq-entry code,
> it prohibits optimizing such kprobes. This means we can
> continuously probing such interrupt handlers by kprobes
> but it is not optimized anymore.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
>  arch/x86/entry/entry_64.S      |    2 +-
>  arch/x86/include/asm/unwind.h  |    1 +
>  arch/x86/kernel/kprobes/opt.c  |    4 ++--
>  arch/x86/kernel/unwind_frame.c |    4 ++--
>  4 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index a9a8027..95bca8b 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -675,7 +675,7 @@ apicinterrupt3 \num trace(\sym) smp_trace(\sym)
>  #endif
>
>  /* Make sure APIC interrupt handlers end up in the irqentry section: */
> -#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
> +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || defined(CONFIG_KPROBES)
>  # define PUSH_SECTION_IRQENTRY .pushsection .irqentry.text, "ax"
>  # define POP_SECTION_IRQENTRY  .popsection
>  #else
> diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
> index e667649..a9896fb9 100644
> --- a/arch/x86/include/asm/unwind.h
> +++ b/arch/x86/include/asm/unwind.h
> @@ -28,6 +28,7 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task,
>  bool unwind_next_frame(struct unwind_state *state);
>
>  unsigned long unwind_get_return_address(struct unwind_state *state);
> +bool in_entry_code(unsigned long ip);
>
>  static inline bool unwind_done(struct unwind_state *state)
>  {
> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
> index 69ea0bc..a51c144 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -39,6 +39,7 @@
>  #include <asm/insn.h>
>  #include <asm/debugreg.h>
>  #include <asm/set_memory.h>
> +#include <asm/unwind.h>
>
>  #include "common.h"
>
> @@ -253,8 +254,7 @@ static int can_optimize(unsigned long paddr)
>          * Do not optimize in the entry code due to the unstable
>          * stack handling.
>          */
> -       if ((paddr >= (unsigned long)__entry_text_start) &&
> -           (paddr <  (unsigned long)__entry_text_end))
> +       if (in_entry_code(paddr))
>                 return 0;
>
>         /* Check there is enough space for a relative jump. */
> diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
> index b9389d7..95123ce 100644
> --- a/arch/x86/kernel/unwind_frame.c
> +++ b/arch/x86/kernel/unwind_frame.c
> @@ -84,14 +84,14 @@ static size_t regs_size(struct pt_regs *regs)
>         return sizeof(*regs);
>  }
>
> -static bool in_entry_code(unsigned long ip)
> +bool in_entry_code(unsigned long ip)
>  {
>         char *addr = (char *)ip;
>
>         if (addr >= __entry_text_start && addr < __entry_text_end)
>                 return true;
>
> -#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)
> +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN) || defined(CONFIG_KPROBES)
>         if (addr >= __irqentry_text_start && addr < __irqentry_text_end)
>                 return true;
>  #endif
> --
> Masami Hiramatsu <mhiramat@kernel.org>




-- 
Francis Deslauriers
Software developer
EfficiOS inc.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web