Commit 6381c24c authored by Masami Hiramatsu's avatar Masami Hiramatsu Committed by Ingo Molnar

kprobes/x86: Fix page-fault handling logic

Current kprobes in-kernel page fault handler doesn't
expect that its single-stepping can be interrupted by
an NMI handler which may cause a page fault(e.g. perf
with callback tracing).

In that case, the page-fault handled by kprobes and it
misunderstands the page-fault has been caused by the
single-stepping code and tries to recover IP address
to probed address.

But the truth is the page-fault has been caused by the
NMI handler, and do_page_fault failes to handle real
page fault because the IP address is modified and
causes Kernel BUGs like below.

 ----
 [ 2264.726905] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
 [ 2264.727190] IP: [<ffffffff813c46e0>] copy_user_generic_string+0x0/0x40

To handle this correctly, I fixed the kprobes fault
handler to ensure the faulted ip address is its own
single-step buffer instead of checking current kprobe
state.
Signed-off-by: default avatarMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Sandeepa Prabhu <sandeepa.prabhu@linaro.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: fche@redhat.com
Cc: systemtap@sourceware.org
Link: http://lkml.kernel.org/r/20140417081644.26341.52351.stgit@ltc230.yrl.intra.hitachi.co.jpSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 6ca2a88a
...@@ -897,9 +897,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) ...@@ -897,9 +897,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
struct kprobe *cur = kprobe_running(); struct kprobe *cur = kprobe_running();
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
switch (kcb->kprobe_status) { if (unlikely(regs->ip == (unsigned long)cur->ainsn.insn)) {
case KPROBE_HIT_SS: /* This must happen on single-stepping */
case KPROBE_REENTER: WARN_ON(kcb->kprobe_status != KPROBE_HIT_SS &&
kcb->kprobe_status != KPROBE_REENTER);
/* /*
* We are here because the instruction being single * We are here because the instruction being single
* stepped caused a page fault. We reset the current * stepped caused a page fault. We reset the current
...@@ -914,9 +915,8 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) ...@@ -914,9 +915,8 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
else else
reset_current_kprobe(); reset_current_kprobe();
preempt_enable_no_resched(); preempt_enable_no_resched();
break; } else if (kcb->kprobe_status == KPROBE_HIT_ACTIVE ||
case KPROBE_HIT_ACTIVE: kcb->kprobe_status == KPROBE_HIT_SSDONE) {
case KPROBE_HIT_SSDONE:
/* /*
* We increment the nmissed count for accounting, * We increment the nmissed count for accounting,
* we can also use npre/npostfault count for accounting * we can also use npre/npostfault count for accounting
...@@ -945,10 +945,8 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) ...@@ -945,10 +945,8 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
* fixup routine could not handle it, * fixup routine could not handle it,
* Let do_page_fault() fix it. * Let do_page_fault() fix it.
*/ */
break;
default:
break;
} }
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment