All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: linux-kernel@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>
Cc: Eddy_Wu@trendmicro.com, x86@kernel.org, davem@davemloft.net,
	rostedt@goodmis.org, naveen.n.rao@linux.ibm.com,
	anil.s.keshavamurthy@intel.com, linux-arch@vger.kernel.org,
	cameron@moodycamel.com, oleg@redhat.com, will@kernel.org,
	paulmck@kernel.org, mhiramat@kernel.org
Subject: [PATCH v4 14/23] kprobes: Remove NMI context check
Date: Fri, 28 Aug 2020 21:29:11 +0900	[thread overview]
Message-ID: <159861775166.992023.8053741091375715032.stgit@devnote2> (raw)
In-Reply-To: <159861759775.992023.12553306821235086809.stgit@devnote2>

Since the commit 9b38cc704e84 ("kretprobe: Prevent triggering
kretprobe from within kprobe_flush_task") sets a dummy current
kprobe in the trampoline handler by kprobe_busy_begin/end(),
it is not possible to run a kretprobe pre handler in kretprobe
trampoline handler context even with the NMI. If the NMI interrupts
a kretprobe_trampoline_handler() and it hits a kretprobe, the
2nd kretprobe will detect recursion correctly and it will be
skipped.
This means we have almost no double-lock issue on kretprobes by NMI.

The last one point is in cleanup_rp_inst() which also takes
kretprobe_table_lock without setting up current kprobes.
So adding kprobe_busy_begin/end() there allows us to remove
in_nmi() check.

The above commit applies kprobe_busy_begin/end() on x86, but
now all arch implementation are unified to generic one, we can
safely remove the in_nmi() check from arch independent code.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/kprobes.c |   16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 46510e5000ff..c8de76d230e3 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1359,7 +1359,8 @@ static void cleanup_rp_inst(struct kretprobe *rp)
 	struct hlist_node *next;
 	struct hlist_head *head;
 
-	/* No race here */
+	/* To avoid recursive kretprobe by NMI, set kprobe busy here */
+	kprobe_busy_begin();
 	for (hash = 0; hash < KPROBE_TABLE_SIZE; hash++) {
 		kretprobe_table_lock(hash, &flags);
 		head = &kretprobe_inst_table[hash];
@@ -1369,6 +1370,8 @@ static void cleanup_rp_inst(struct kretprobe *rp)
 		}
 		kretprobe_table_unlock(hash, &flags);
 	}
+	kprobe_busy_end();
+
 	free_rp_inst(rp);
 }
 NOKPROBE_SYMBOL(cleanup_rp_inst);
@@ -2033,17 +2036,6 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
 	unsigned long hash, flags = 0;
 	struct kretprobe_instance *ri;
 
-	/*
-	 * To avoid deadlocks, prohibit return probing in NMI contexts,
-	 * just skip the probe and increase the (inexact) 'nmissed'
-	 * statistical counter, so that the user is informed that
-	 * something happened:
-	 */
-	if (unlikely(in_nmi())) {
-		rp->nmissed++;
-		return 0;
-	}
-
 	/* TODO: consider to only swap the RA after the last pre_handler fired */
 	hash = hash_ptr(current, KPROBE_HASH_BITS);
 	raw_spin_lock_irqsave(&rp->lock, flags);


  parent reply	other threads:[~2020-08-28 12:29 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-28 12:26 [PATCH v4 00/23] kprobes: Unify kretprobe trampoline handlers and make kretprobe lockless Masami Hiramatsu
2020-08-28 12:26 ` [PATCH v4 01/23] kprobes: Add generic kretprobe trampoline handler Masami Hiramatsu
2020-08-28 12:27 ` [PATCH v4 02/23] x86/kprobes: Use " Masami Hiramatsu
2020-08-28 12:27 ` [PATCH v4 03/23] arm: kprobes: " Masami Hiramatsu
2020-08-28 12:27 ` [PATCH v4 04/23] arm64: " Masami Hiramatsu
2020-08-28 13:31   ` Mark Rutland
2020-08-28 13:37     ` peterz
2020-08-28 13:48       ` Mark Rutland
2020-08-28 13:58       ` Masami Hiramatsu
2020-08-28 12:27 ` [PATCH v4 05/23] arc: " Masami Hiramatsu
2020-08-28 12:27 ` [PATCH v4 06/23] csky: " Masami Hiramatsu
2020-08-28 12:27 ` [PATCH v4 07/23] ia64: " Masami Hiramatsu
2020-08-28 12:28 ` [PATCH v4 08/23] mips: " Masami Hiramatsu
2020-08-28 12:28 ` [PATCH v4 09/23] parisc: " Masami Hiramatsu
2020-08-28 12:28 ` [PATCH v4 10/23] powerpc: " Masami Hiramatsu
2020-08-28 12:28 ` [PATCH v4 11/23] s390: " Masami Hiramatsu
2020-08-28 12:28 ` [PATCH v4 12/23] sh: " Masami Hiramatsu
2020-08-28 12:29 ` [PATCH v4 13/23] sparc: " Masami Hiramatsu
2020-08-28 12:29 ` Masami Hiramatsu [this message]
2020-08-28 12:29 ` [PATCH v4 15/23] kprobes: Free kretprobe_instance with rcu callback Masami Hiramatsu
2020-08-28 12:29 ` [PATCH v4 16/23] kprobes: Make local used functions static Masami Hiramatsu
2020-08-28 12:29 ` [PATCH v4 17/23] llist: Add nonatomic __llist_add() Masami Hiramatsu
2020-08-28 12:29 ` [PATCH v4 18/23] sched: Fix try_invoke_on_locked_down_task() semantics Masami Hiramatsu
2020-08-29  2:01   ` Masami Hiramatsu
2020-08-29  7:30     ` peterz
2020-08-29 17:31       ` Paul E. McKenney
2020-08-28 12:30 ` [PATCH v4 19/23] kprobes: Remove kretprobe hash Masami Hiramatsu
2020-08-28 18:37   ` Masami Hiramatsu
2020-08-28 19:02     ` peterz
2020-08-28 19:32   ` Eddy_Wu
2020-08-28 20:29     ` Peter Zijlstra
2020-08-29  1:23       ` Masami Hiramatsu
2020-08-28 12:30 ` [PATCH v4 20/23] [RFC] kprobes: Remove task scan for updating kretprobe_instance Masami Hiramatsu
2020-08-28 12:52   ` peterz
2020-08-28 15:10     ` Masami Hiramatsu
2020-08-28 15:18       ` peterz
2020-08-28 16:01         ` Masami Hiramatsu
2020-08-28 12:30 ` [PATCH v4 21/23] asm-generic/atomic: Add try_cmpxchg() fallbacks Masami Hiramatsu
2020-08-28 12:30 ` [PATCH v4 22/23] freelist: Lock less freelist Masami Hiramatsu
2020-08-28 12:30 ` [PATCH v4 23/23] kprobes: Replace rp->free_instance with freelist Masami Hiramatsu
2020-08-28 12:37 ` [PATCH v4 00/23] kprobes: Unify kretprobe trampoline handlers and make kretprobe lockless 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=159861775166.992023.8053741091375715032.stgit@devnote2 \
    --to=mhiramat@kernel.org \
    --cc=Eddy_Wu@trendmicro.com \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=cameron@moodycamel.com \
    --cc=davem@davemloft.net \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=oleg@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.