From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752279AbdBJWwc (ORCPT ); Fri, 10 Feb 2017 17:52:32 -0500 Received: from mail.kernel.org ([198.145.29.136]:60752 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751038AbdBJWwb (ORCPT ); Fri, 10 Feb 2017 17:52:31 -0500 Date: Sat, 11 Feb 2017 07:52:19 +0900 From: Masami Hiramatsu To: Masami Hiramatsu Cc: Ingo Molnar , linux-kernel@vger.kernel.org, Peter Zijlstra , Ananth N Mavinakayanahalli , Thomas Gleixner , "H . Peter Anvin" , Jon Medhurst , Wang Nan , Russell King , Catalin Marinas , Will Deacon , "David A . Long" , Sandeepa Prabhu Subject: Re: [BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible deadlock in kretprobe Message-Id: <20170211075219.cdab444239319f272cb75998@kernel.org> In-Reply-To: <148665771695.22817.16393459806489781531.stgit@devbox> References: <148665771695.22817.16393459806489781531.stgit@devbox> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Feb 2017 01:28:47 +0900 Masami Hiramatsu wrote: > Hi, > > Here is 2nd version of the series. I just updated the > patch description to make it easier to understand > on arm and arm64, no code change. > > V1 is here: > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1327856.html > > ---- > This series will fix a possible deadlock case in kretprobe > on x86, arm, arm64. Since kretprobe has been optimized on > those arch, they have similar possible deadlock issue. > > Problem > ===== > > The deadlock senario is when a user puts 2 kretprobes, > one on normal function and one on a function which can be > called from NMI or FIQ where normal interrupt disabled. > (we don't recommend it, but possible.) 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 NMI(or FIQ on arm/arm64) is occurred and > 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 on the > spinlock. > Note that this is very rare case, but theoretically happens. Please ignore this series. I found that the kretprobe already rejected probes in NMI (and FIQ) so this senario never be true (as far as in_nmi() works in nested interrupt context.) Sorry for bother you. And thank you! > > Reason and Affected Arch > ===== > > 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. So, currently only x86, arm, and arm64 are > affected, because other arch have not implemented the > kretprobe-booster. > > Solution > ===== > > 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 an NMI/FIQ occurred and 2nd kretprobe's kprobe > is kicked (to modify the return address, a kprobe is kicked > when the target function is called), the 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 kprobe on trampoline code. This fix just revived it. > > Thank you, > > --- > > Masami Hiramatsu (3): > kprobes/x86: Fix a possible deadlock case in kretprobe > kprobes/arm64: Fix a possible deadlock case in kretprobe > kprobes/arm: Fix a possible deadlock case in kretprobe > > > arch/arm/probes/kprobes/core.c | 12 ++++++++++-- > arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++-- > arch/x86/kernel/kprobes/core.c | 13 ++++++++++--- > 3 files changed, 30 insertions(+), 7 deletions(-) > > -- > Masami Hiramatsu -- Masami Hiramatsu