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

Groups > linux.kernel > #1577771

[BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible deadlock case in kretprobe

Show all headers | View raw

Fix a possibility of deadlock case in kretprobe on arm
implementation. There may be a chance that the kretprobe
hash table lock can cause a dead lock.

The senario is that a user puts 2 kretprobes, one on normal
function and one on a function which can be called from
somewhare which can interrupt in irq disabled critical
section like FIQ.

In this case, if the kernel hits the 1st kretprobe on a
normal function return which calls trampoline_handler(),
acquire a spinlock on the hash table in kretprobe_hash_lock()
and disable irqs. After that, if the 2nd kretprobe is kicked
from FIQ, it also calls trampoline_handler() and tries to
acquire the same spinlock (since the hash is based on
current task, same as the 1st kretprobe), it causes
a deadlock.

Actually, this bug has been introduced by kretprobe-booster
which removes a kprobe from return trampoline code, but also
resets current kprobe, which can be a stopper for the nested
k(ret)probes.

To fix this issue, I introduced a dummy kprobe which is set
as a current kprobe while holding the kretprobe-hash lock.
With that, if the 2nd kretprobe's kprobe is kicked
(to modify the return address, a kprobe is kicked when
the target function is called), that kprobe (and the 2nd
kretprobe also) is skipped because it detects there is
another kprobe is running.

This reentrance detection and nested kprobe blocker had
existed when the original kretprobe was implemented by
using a kprobe on trampoline code. This fixes just revived it.


Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm/probes/kprobes/core.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index b6dc9d8..3e5aab7 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -426,6 +426,8 @@ void __naked __kprobes kretprobe_trampoline(void)
 		: : : "memory");
 }
 
+static struct kprobe dummy_retprobe = {.addr = (void *)&kretprobe_trampoline};
+
 /* Called from kretprobe_trampoline */
 static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
 {
@@ -436,6 +438,11 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
 	unsigned long trampoline_address = (unsigned long)&kretprobe_trampoline;
 
 	INIT_HLIST_HEAD(&empty_rp);
+
+	/* This prevents kernel to change running cpu while processing */
+	preempt_disable();
+	get_kprobe_ctlblk()->kprobe_status = KPROBE_HIT_ACTIVE;
+	__this_cpu_write(current_kprobe, &dummy_retprobe);
 	kretprobe_hash_lock(current, &head, &flags);
 
 	/*
@@ -458,9 +465,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
 
 		if (ri->rp && ri->rp->handler) {
 			__this_cpu_write(current_kprobe, &ri->rp->kp);
-			get_kprobe_ctlblk()->kprobe_status = KPROBE_HIT_ACTIVE;
 			ri->rp->handler(ri, regs);
-			__this_cpu_write(current_kprobe, NULL);
+			__this_cpu_write(current_kprobe, &dummy_retprobe);
 		}
 
 		orig_ret_address = (unsigned long)ri->ret_addr;
@@ -477,6 +483,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
 
 	kretprobe_assert(ri, orig_ret_address, trampoline_address);
 	kretprobe_hash_unlock(current, &flags);
+	__this_cpu_write(current_kprobe, NULL);
+	preempt_enable_no_resched();
 
 	hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
 		hlist_del(&ri->hlist);

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible deadlock in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-09 17:40 +0100
  [BUGFIX PATCH tip/master V2 2/3] kprobes/arm64: Fix a possible deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-09 17:40 +0100
  [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-09 17:50 +0100
    Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-09 18:30 +0100
      Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-10 03:40 +0100
        Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-10 23:40 +0100
          Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-11 00:30 +0100
            Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-11 10:30 +0100
  Re: [BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible  deadlock in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-11 00:00 +0100

csiph-web