From: Masami Hiramatsu <mhiramat@kernel.org>
To: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
Thomas Gleixner <tglx@linutronix.de>,
"H . Peter Anvin" <hpa@zytor.com>, Jon Medhurst <tixy@linaro.org>,
Wang Nan <wangnan0@huawei.com>,
Russell King <linux@armlinux.org.uk>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
"David A . Long" <dave.long@linaro.org>,
Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>
Subject: [BUGFIX PATCH tip/master V2 2/3] kprobes/arm64: Fix a possible deadlock case in kretprobe
Date: Fri, 10 Feb 2017 01:31:11 +0900 [thread overview]
Message-ID: <148665786139.22817.6397474944263297987.stgit@devbox> (raw)
In-Reply-To: <148665771695.22817.16393459806489781531.stgit@devbox>
Fix a possibility of deadlock case in kretprobe on arm64
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 hw-breakpoint, FIQ in the future, etc.
(At this point those code should be protected from kprobes.)
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,
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.
Note that again, this must not happen at this moment, but
if we support FIQ, it can happen.
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. On arm64, kretprobe-booster has been ported
from x86.
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 on x86 was implemented by
using a kprobe on trampoline code. This fixes just revived it.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 2a07aae..401f7c9 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -561,6 +561,8 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
return false;
}
+static struct kprobe dummy_retprobe = {.addr = (void *)&kretprobe_trampoline};
+
void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
{
struct kretprobe_instance *ri = NULL;
@@ -572,6 +574,11 @@ void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
kprobe_opcode_t *correct_ret_addr = NULL;
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);
/*
@@ -614,10 +621,9 @@ void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
orig_ret_address = (unsigned long)ri->ret_addr;
if (ri->rp && ri->rp->handler) {
__this_cpu_write(current_kprobe, &ri->rp->kp);
- get_kprobe_ctlblk()->kprobe_status = KPROBE_HIT_ACTIVE;
ri->ret_addr = correct_ret_addr;
ri->rp->handler(ri, regs);
- __this_cpu_write(current_kprobe, NULL);
+ __this_cpu_write(current_kprobe, &dummy_retprobe);
}
recycle_rp_inst(ri, &empty_rp);
@@ -632,6 +638,8 @@ void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
}
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);
next prev parent reply other threads:[~2017-02-09 16:33 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-09 16:28 [BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible deadlock in kretprobe Masami Hiramatsu
2017-02-09 16:30 ` [BUGFIX PATCH tip/master V2 1/3] kprobes/x86: Fix a possible deadlock case " Masami Hiramatsu
2017-02-09 16:31 ` Masami Hiramatsu [this message]
2017-02-09 16:32 ` [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: " Masami Hiramatsu
2017-02-09 16:49 ` Russell King - ARM Linux
2017-02-10 2:34 ` Masami Hiramatsu
2017-02-10 22:33 ` Masami Hiramatsu
2017-02-10 23:21 ` Russell King - ARM Linux
2017-02-11 9:21 ` Masami Hiramatsu
2017-02-10 22:52 ` [BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible deadlock " Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=148665786139.22817.6397474944263297987.stgit@devbox \
--to=mhiramat@kernel.org \
--cc=ananth@linux.vnet.ibm.com \
--cc=catalin.marinas@arm.com \
--cc=dave.long@linaro.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=sandeepa.s.prabhu@gmail.com \
--cc=tglx@linutronix.de \
--cc=tixy@linaro.org \
--cc=wangnan0@huawei.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox