All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups
@ 2020-05-12  8:02 Masami Hiramatsu
  2020-05-12  8:02 ` [PATCH -tip V6 1/6] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

Hi Ingo,

Here is the 6th version of the series for kprobes. The previous
version is here.

 https://lore.kernel.org/lkml/158583483116.26060.10517933482238348979.stgit@devnote2/

In this version, I picked 2 patches[1][2] which has been reviewed
on LKML but not merged to tip tree yet.

[1] https://lore.kernel.org/lkml/20200408164641.3299633-1-jolsa@kernel.org/
[2] https://lore.kernel.org/lkml/20200507185733.GA14931@embeddedor/

You can also pull this series from kprobes/core branch from

 https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/

Thank you,

---

Gustavo A. R. Silva (1):
      kprobes: Replace zero-length array with flexible-array

Jiri Olsa (1):
      kretprobe: Prevent triggering kretprobe from within kprobe_flush_task

Masami Hiramatsu (4):
      kprobes: Suppress the suspicious RCU warning on kprobes
      kprobes: Use non RCU traversal APIs on kprobe_tables if possible
      kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex
      kprobes: Remove redundant arch_disarm_kprobe() call


 arch/x86/kernel/kprobes/core.c |   16 ++--------
 include/linux/kprobes.h        |    6 +++-
 kernel/kprobes.c               |   61 +++++++++++++++++++++++++++++++---------
 3 files changed, 56 insertions(+), 27 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH -tip V6 1/6] kprobes: Suppress the suspicious RCU warning on kprobes
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
@ 2020-05-12  8:02 ` Masami Hiramatsu
  2020-05-12  8:02 ` [PATCH -tip V6 2/6] kprobes: Use non RCU traversal APIs on kprobe_tables if possible Masami Hiramatsu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

Anders reported that the lockdep warns that suspicious
RCU list usage in register_kprobe() (detected by
CONFIG_PROVE_RCU_LIST.) This is because get_kprobe()
access kprobe_table[] by hlist_for_each_entry_rcu()
without rcu_read_lock.

If we call get_kprobe() from the breakpoint handler context,
it is run with preempt disabled, so this is not a problem.
But in other cases, instead of rcu_read_lock(), we locks
kprobe_mutex so that the kprobe_table[] is not updated.
So, current code is safe, but still not good from the view
point of RCU.

Joel suggested that we can silent that warning by passing
lockdep_is_held() to the last argument of
hlist_for_each_entry_rcu().

Add lockdep_is_held(&kprobe_mutex) at the end of the
hlist_for_each_entry_rcu() to suppress the warning.

Reported-by: Anders Roxell <anders.roxell@linaro.org>
Suggested-by: Joel Fernandes <joel@joelfernandes.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/kprobes.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2625c241ac00..bd484392d789 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -326,7 +326,8 @@ struct kprobe *get_kprobe(void *addr)
 	struct kprobe *p;
 
 	head = &kprobe_table[hash_ptr(addr, KPROBE_HASH_BITS)];
-	hlist_for_each_entry_rcu(p, head, hlist) {
+	hlist_for_each_entry_rcu(p, head, hlist,
+				 lockdep_is_held(&kprobe_mutex)) {
 		if (p->addr == addr)
 			return p;
 	}


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -tip V6 2/6] kprobes: Use non RCU traversal APIs on kprobe_tables if possible
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
  2020-05-12  8:02 ` [PATCH -tip V6 1/6] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
@ 2020-05-12  8:02 ` Masami Hiramatsu
  2020-05-12  8:02 ` [PATCH -tip V6 3/6] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex Masami Hiramatsu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

Current kprobes uses RCU traversal APIs on kprobe_tables
even if it is safe because kprobe_mutex is locked.

Make those traversals to non-RCU APIs where the kprobe_mutex
is locked.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/kprobes.c |   29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index bd484392d789..38d9a5d7c8a4 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -46,6 +46,11 @@
 
 
 static int kprobes_initialized;
+/* kprobe_table can be accessed by
+ * - Normal hlist traversal and RCU add/del under kprobe_mutex is held.
+ * Or
+ * - RCU hlist traversal under disabling preempt (breakpoint handlers)
+ */
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 
@@ -850,7 +855,7 @@ static void optimize_all_kprobes(void)
 	kprobes_allow_optimization = true;
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
-		hlist_for_each_entry_rcu(p, head, hlist)
+		hlist_for_each_entry(p, head, hlist)
 			if (!kprobe_disabled(p))
 				optimize_kprobe(p);
 	}
@@ -877,7 +882,7 @@ static void unoptimize_all_kprobes(void)
 	kprobes_allow_optimization = false;
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
-		hlist_for_each_entry_rcu(p, head, hlist) {
+		hlist_for_each_entry(p, head, hlist) {
 			if (!kprobe_disabled(p))
 				unoptimize_kprobe(p, false);
 		}
@@ -1500,12 +1505,14 @@ static struct kprobe *__get_valid_kprobe(struct kprobe *p)
 {
 	struct kprobe *ap, *list_p;
 
+	lockdep_assert_held(&kprobe_mutex);
+
 	ap = get_kprobe(p->addr);
 	if (unlikely(!ap))
 		return NULL;
 
 	if (p != ap) {
-		list_for_each_entry_rcu(list_p, &ap->list, list)
+		list_for_each_entry(list_p, &ap->list, list)
 			if (list_p == p)
 			/* kprobe p is a valid probe */
 				goto valid;
@@ -1670,7 +1677,9 @@ static int aggr_kprobe_disabled(struct kprobe *ap)
 {
 	struct kprobe *kp;
 
-	list_for_each_entry_rcu(kp, &ap->list, list)
+	lockdep_assert_held(&kprobe_mutex);
+
+	list_for_each_entry(kp, &ap->list, list)
 		if (!kprobe_disabled(kp))
 			/*
 			 * There is an active probe on the list.
@@ -1749,7 +1758,7 @@ static int __unregister_kprobe_top(struct kprobe *p)
 	else {
 		/* If disabling probe has special handlers, update aggrprobe */
 		if (p->post_handler && !kprobe_gone(p)) {
-			list_for_each_entry_rcu(list_p, &ap->list, list) {
+			list_for_each_entry(list_p, &ap->list, list) {
 				if ((list_p != p) && (list_p->post_handler))
 					goto noclean;
 			}
@@ -2063,13 +2072,15 @@ static void kill_kprobe(struct kprobe *p)
 {
 	struct kprobe *kp;
 
+	lockdep_assert_held(&kprobe_mutex);
+
 	p->flags |= KPROBE_FLAG_GONE;
 	if (kprobe_aggrprobe(p)) {
 		/*
 		 * If this is an aggr_kprobe, we have to list all the
 		 * chained probes and mark them GONE.
 		 */
-		list_for_each_entry_rcu(kp, &p->list, list)
+		list_for_each_entry(kp, &p->list, list)
 			kp->flags |= KPROBE_FLAG_GONE;
 		p->post_handler = NULL;
 		kill_optimized_kprobe(p);
@@ -2238,7 +2249,7 @@ static int kprobes_module_callback(struct notifier_block *nb,
 	mutex_lock(&kprobe_mutex);
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
-		hlist_for_each_entry_rcu(p, head, hlist)
+		hlist_for_each_entry(p, head, hlist)
 			if (within_module_init((unsigned long)p->addr, mod) ||
 			    (checkcore &&
 			     within_module_core((unsigned long)p->addr, mod))) {
@@ -2489,7 +2500,7 @@ static int arm_all_kprobes(void)
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
 		/* Arm all kprobes on a best-effort basis */
-		hlist_for_each_entry_rcu(p, head, hlist) {
+		hlist_for_each_entry(p, head, hlist) {
 			if (!kprobe_disabled(p)) {
 				err = arm_kprobe(p);
 				if (err)  {
@@ -2532,7 +2543,7 @@ static int disarm_all_kprobes(void)
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
 		/* Disarm all kprobes on a best-effort basis */
-		hlist_for_each_entry_rcu(p, head, hlist) {
+		hlist_for_each_entry(p, head, hlist) {
 			if (!arch_trampoline_kprobe(p) && !kprobe_disabled(p)) {
 				err = disarm_kprobe(p, false);
 				if (err) {


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -tip V6 3/6] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
  2020-05-12  8:02 ` [PATCH -tip V6 1/6] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
  2020-05-12  8:02 ` [PATCH -tip V6 2/6] kprobes: Use non RCU traversal APIs on kprobe_tables if possible Masami Hiramatsu
@ 2020-05-12  8:02 ` Masami Hiramatsu
  2020-05-12  8:03 ` [PATCH -tip V6 4/6] kprobes: Remove redundant arch_disarm_kprobe() call Masami Hiramatsu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

In kprobe_optimizer() kick_kprobe_optimizer() is called
without kprobe_mutex, but this can race with other caller
which is protected by kprobe_mutex.

To fix that, expand kprobe_mutex protected area to protect
kick_kprobe_optimizer() call.

Fixes: cd7ebe2298ff ("kprobes: Use text_poke_smp_batch for optimizing")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
  Changes in v5: Add Fixes and Cc:stable tags.
---
 kernel/kprobes.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 38d9a5d7c8a4..6d76a6e3e1a5 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -592,11 +592,12 @@ static void kprobe_optimizer(struct work_struct *work)
 	mutex_unlock(&module_mutex);
 	mutex_unlock(&text_mutex);
 	cpus_read_unlock();
-	mutex_unlock(&kprobe_mutex);
 
 	/* Step 5: Kick optimizer again if needed */
 	if (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list))
 		kick_kprobe_optimizer();
+
+	mutex_unlock(&kprobe_mutex);
 }
 
 /* Wait for completing optimization and unoptimization */


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -tip V6 4/6] kprobes: Remove redundant arch_disarm_kprobe() call
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2020-05-12  8:02 ` [PATCH -tip V6 3/6] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex Masami Hiramatsu
@ 2020-05-12  8:03 ` Masami Hiramatsu
  2020-05-12  8:03 ` [PATCH -tip V6 5/6] kretprobe: Prevent triggering kretprobe from within kprobe_flush_task Masami Hiramatsu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

Fix to remove redundant arch_disarm_kprobe() call in
force_unoptimize_kprobe(). This arch_disarm_kprobe()
will be invoked if the kprobe is optimized but disabled,
but that means the kprobe (optprobe) is unused (and
unoptimized) state.

In that case, unoptimize_kprobe() puts it in freeing_list
and kprobe_optimizer (do_unoptimize_kprobes()) automatically
disarm it. Thus this arch_disarm_kprobe() is redundant.

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

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 6d76a6e3e1a5..627fc1b7011a 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -675,8 +675,6 @@ static void force_unoptimize_kprobe(struct optimized_kprobe *op)
 	lockdep_assert_cpus_held();
 	arch_unoptimize_kprobe(op);
 	op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
-	if (kprobe_disabled(&op->kp))
-		arch_disarm_kprobe(&op->kp);
 }
 
 /* Unoptimize a kprobe if p is optimized */


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -tip V6 5/6] kretprobe: Prevent triggering kretprobe from within kprobe_flush_task
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2020-05-12  8:03 ` [PATCH -tip V6 4/6] kprobes: Remove redundant arch_disarm_kprobe() call Masami Hiramatsu
@ 2020-05-12  8:03 ` Masami Hiramatsu
  2020-05-12  8:03 ` [PATCH -tip V6 6/6] kprobes: Replace zero-length array with flexible-array Masami Hiramatsu
  2020-05-27 14:49 ` [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
  6 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

From: Jiri Olsa <jolsa@redhat.com>

Ziqian reported lockup when adding retprobe on _raw_spin_lock_irqsave.
My test was also able to trigger lockdep output:

 ============================================
 WARNING: possible recursive locking detected
 5.6.0-rc6+ #6 Not tainted
 --------------------------------------------
 sched-messaging/2767 is trying to acquire lock:
 ffffffff9a492798 (&(kretprobe_table_locks[i].lock)){-.-.}, at: kretprobe_hash_lock+0x52/0xa0

 but task is already holding lock:
 ffffffff9a491a18 (&(kretprobe_table_locks[i].lock)){-.-.}, at: kretprobe_trampoline+0x0/0x50

 other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&(kretprobe_table_locks[i].lock));
   lock(&(kretprobe_table_locks[i].lock));

  *** DEADLOCK ***

  May be due to missing lock nesting notation

 1 lock held by sched-messaging/2767:
  #0: ffffffff9a491a18 (&(kretprobe_table_locks[i].lock)){-.-.}, at: kretprobe_trampoline+0x0/0x50

 stack backtrace:
 CPU: 3 PID: 2767 Comm: sched-messaging Not tainted 5.6.0-rc6+ #6
 Call Trace:
  dump_stack+0x96/0xe0
  __lock_acquire.cold.57+0x173/0x2b7
  ? native_queued_spin_lock_slowpath+0x42b/0x9e0
  ? lockdep_hardirqs_on+0x590/0x590
  ? __lock_acquire+0xf63/0x4030
  lock_acquire+0x15a/0x3d0
  ? kretprobe_hash_lock+0x52/0xa0
  _raw_spin_lock_irqsave+0x36/0x70
  ? kretprobe_hash_lock+0x52/0xa0
  kretprobe_hash_lock+0x52/0xa0
  trampoline_handler+0xf8/0x940
  ? kprobe_fault_handler+0x380/0x380
  ? find_held_lock+0x3a/0x1c0
  kretprobe_trampoline+0x25/0x50
  ? lock_acquired+0x392/0xbc0
  ? _raw_spin_lock_irqsave+0x50/0x70
  ? __get_valid_kprobe+0x1f0/0x1f0
  ? _raw_spin_unlock_irqrestore+0x3b/0x40
  ? finish_task_switch+0x4b9/0x6d0
  ? __switch_to_asm+0x34/0x70
  ? __switch_to_asm+0x40/0x70

The code within the kretprobe handler checks for probe reentrancy,
so we won't trigger any _raw_spin_lock_irqsave probe in there.

The problem is in outside kprobe_flush_task, where we call:

  kprobe_flush_task
    kretprobe_table_lock
      raw_spin_lock_irqsave
        _raw_spin_lock_irqsave

where _raw_spin_lock_irqsave triggers the kretprobe and installs
kretprobe_trampoline handler on _raw_spin_lock_irqsave return.

The kretprobe_trampoline handler is then executed with already
locked kretprobe_table_locks, and first thing it does is to
lock kretprobe_table_locks ;-) the whole lockup path like:

  kprobe_flush_task
    kretprobe_table_lock
      raw_spin_lock_irqsave
        _raw_spin_lock_irqsave ---> probe triggered, kretprobe_trampoline installed

        ---> kretprobe_table_locks locked

        kretprobe_trampoline
          trampoline_handler
            kretprobe_hash_lock(current, &head, &flags);  <--- deadlock

Adding kprobe_busy_begin/end helpers that mark code with fake
probe installed to prevent triggering of another kprobe within
this code.

Using these helpers in kprobe_flush_task, so the probe recursion
protection check is hit and the probe is never set to prevent
above lockup.

Fixes: ef53d9c5e4da ("kprobes: improve kretprobe scalability with hashed locking")
Cc: stable@vger.kernel.org
Reported-by: "Ziqian SUN (Zamir)" <zsun@redhat.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/kernel/kprobes/core.c |   16 +++-------------
 include/linux/kprobes.h        |    4 ++++
 kernel/kprobes.c               |   24 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 4d7022a740ab..a12adbe1559d 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -753,16 +753,11 @@ asm(
 NOKPROBE_SYMBOL(kretprobe_trampoline);
 STACK_FRAME_NON_STANDARD(kretprobe_trampoline);
 
-static struct kprobe kretprobe_kprobe = {
-	.addr = (void *)kretprobe_trampoline,
-};
-
 /*
  * Called from kretprobe_trampoline
  */
 __used __visible void *trampoline_handler(struct pt_regs *regs)
 {
-	struct kprobe_ctlblk *kcb;
 	struct kretprobe_instance *ri = NULL;
 	struct hlist_head *head, empty_rp;
 	struct hlist_node *tmp;
@@ -772,16 +767,12 @@ __used __visible void *trampoline_handler(struct pt_regs *regs)
 	void *frame_pointer;
 	bool skipped = false;
 
-	preempt_disable();
-
 	/*
 	 * Set a dummy kprobe for avoiding kretprobe recursion.
 	 * Since kretprobe never run in kprobe handler, kprobe must not
 	 * be running at this point.
 	 */
-	kcb = get_kprobe_ctlblk();
-	__this_cpu_write(current_kprobe, &kretprobe_kprobe);
-	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+	kprobe_busy_begin();
 
 	INIT_HLIST_HEAD(&empty_rp);
 	kretprobe_hash_lock(current, &head, &flags);
@@ -857,7 +848,7 @@ __used __visible void *trampoline_handler(struct pt_regs *regs)
 			__this_cpu_write(current_kprobe, &ri->rp->kp);
 			ri->ret_addr = correct_ret_addr;
 			ri->rp->handler(ri, regs);
-			__this_cpu_write(current_kprobe, &kretprobe_kprobe);
+			__this_cpu_write(current_kprobe, &kprobe_busy);
 		}
 
 		recycle_rp_inst(ri, &empty_rp);
@@ -873,8 +864,7 @@ __used __visible void *trampoline_handler(struct pt_regs *regs)
 
 	kretprobe_hash_unlock(current, &flags);
 
-	__this_cpu_write(current_kprobe, NULL);
-	preempt_enable();
+	kprobe_busy_end();
 
 	hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
 		hlist_del(&ri->hlist);
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 04bdaf01112c..645fd401c856 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -350,6 +350,10 @@ static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
 	return this_cpu_ptr(&kprobe_ctlblk);
 }
 
+extern struct kprobe kprobe_busy;
+void kprobe_busy_begin(void);
+void kprobe_busy_end(void);
+
 kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset);
 int register_kprobe(struct kprobe *p);
 void unregister_kprobe(struct kprobe *p);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 627fc1b7011a..3cb9e29908ed 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1241,6 +1241,26 @@ __releases(hlist_lock)
 }
 NOKPROBE_SYMBOL(kretprobe_table_unlock);
 
+struct kprobe kprobe_busy = {
+	.addr = (void *) get_kprobe,
+};
+
+void kprobe_busy_begin(void)
+{
+	struct kprobe_ctlblk *kcb;
+
+	preempt_disable();
+	__this_cpu_write(current_kprobe, &kprobe_busy);
+	kcb = get_kprobe_ctlblk();
+	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+}
+
+void kprobe_busy_end(void)
+{
+	__this_cpu_write(current_kprobe, NULL);
+	preempt_enable();
+}
+
 /*
  * This function is called from finish_task_switch when task tk becomes dead,
  * so that we can recycle any function-return probe instances associated
@@ -1258,6 +1278,8 @@ void kprobe_flush_task(struct task_struct *tk)
 		/* Early boot.  kretprobe_table_locks not yet initialized. */
 		return;
 
+	kprobe_busy_begin();
+
 	INIT_HLIST_HEAD(&empty_rp);
 	hash = hash_ptr(tk, KPROBE_HASH_BITS);
 	head = &kretprobe_inst_table[hash];
@@ -1271,6 +1293,8 @@ void kprobe_flush_task(struct task_struct *tk)
 		hlist_del(&ri->hlist);
 		kfree(ri);
 	}
+
+	kprobe_busy_end();
 }
 NOKPROBE_SYMBOL(kprobe_flush_task);
 


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -tip V6 6/6] kprobes: Replace zero-length array with flexible-array
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2020-05-12  8:03 ` [PATCH -tip V6 5/6] kretprobe: Prevent triggering kretprobe from within kprobe_flush_task Masami Hiramatsu
@ 2020-05-12  8:03 ` Masami Hiramatsu
  2020-05-27 14:49 ` [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
  6 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-12  8:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Linux Kernel Mailing List, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN

From: Gustavo A. R. Silva <gustavoars@kernel.org>

The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/kprobes.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 645fd401c856..53c4f2c1d658 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -161,7 +161,7 @@ struct kretprobe_instance {
 	kprobe_opcode_t *ret_addr;
 	struct task_struct *task;
 	void *fp;
-	char data[0];
+	char data[];
 };
 
 struct kretprobe_blackpoint {


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups
  2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
                   ` (5 preceding siblings ...)
  2020-05-12  8:03 ` [PATCH -tip V6 6/6] kprobes: Replace zero-length array with flexible-array Masami Hiramatsu
@ 2020-05-27 14:49 ` Masami Hiramatsu
  2020-06-14 15:37   ` Jiri Olsa
  6 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2020-05-27 14:49 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar,
	Peter Zijlstra, Ziqian SUN, Jiri Olsa

(Oops, I missed Jiri in loop.)

Hi Ingo,

Could you take this series?
These are not adding any feature, but fixing real bugs.

Thank you,

On Tue, 12 May 2020 17:02:22 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi Ingo,
> 
> Here is the 6th version of the series for kprobes. The previous
> version is here.
> 
>  https://lore.kernel.org/lkml/158583483116.26060.10517933482238348979.stgit@devnote2/
> 
> In this version, I picked 2 patches[1][2] which has been reviewed
> on LKML but not merged to tip tree yet.
> 
> [1] https://lore.kernel.org/lkml/20200408164641.3299633-1-jolsa@kernel.org/
> [2] https://lore.kernel.org/lkml/20200507185733.GA14931@embeddedor/
> 
> You can also pull this series from kprobes/core branch from
> 
>  https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/
> 
> Thank you,
> 
> ---
> 
> Gustavo A. R. Silva (1):
>       kprobes: Replace zero-length array with flexible-array
> 
> Jiri Olsa (1):
>       kretprobe: Prevent triggering kretprobe from within kprobe_flush_task
> 
> Masami Hiramatsu (4):
>       kprobes: Suppress the suspicious RCU warning on kprobes
>       kprobes: Use non RCU traversal APIs on kprobe_tables if possible
>       kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex
>       kprobes: Remove redundant arch_disarm_kprobe() call
> 
> 
>  arch/x86/kernel/kprobes/core.c |   16 ++--------
>  include/linux/kprobes.h        |    6 +++-
>  kernel/kprobes.c               |   61 +++++++++++++++++++++++++++++++---------
>  3 files changed, 56 insertions(+), 27 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups
  2020-05-27 14:49 ` [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
@ 2020-06-14 15:37   ` Jiri Olsa
  2020-06-16 19:01     ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2020-06-14 15:37 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Gustavo A . R . Silva, Anders Roxell, paulmck, joel,
	Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar,
	Peter Zijlstra, Ziqian SUN, Jiri Olsa

On Wed, May 27, 2020 at 11:49:41PM +0900, Masami Hiramatsu wrote:
> (Oops, I missed Jiri in loop.)
> 
> Hi Ingo,
> 
> Could you take this series?
> These are not adding any feature, but fixing real bugs.

Hi,
I still can't see this being pulled in, did I miss it?

thanks,
jirka

> 
> Thank you,
> 
> On Tue, 12 May 2020 17:02:22 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Hi Ingo,
> > 
> > Here is the 6th version of the series for kprobes. The previous
> > version is here.
> > 
> >  https://lore.kernel.org/lkml/158583483116.26060.10517933482238348979.stgit@devnote2/
> > 
> > In this version, I picked 2 patches[1][2] which has been reviewed
> > on LKML but not merged to tip tree yet.
> > 
> > [1] https://lore.kernel.org/lkml/20200408164641.3299633-1-jolsa@kernel.org/
> > [2] https://lore.kernel.org/lkml/20200507185733.GA14931@embeddedor/
> > 
> > You can also pull this series from kprobes/core branch from
> > 
> >  https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/
> > 
> > Thank you,
> > 
> > ---
> > 
> > Gustavo A. R. Silva (1):
> >       kprobes: Replace zero-length array with flexible-array
> > 
> > Jiri Olsa (1):
> >       kretprobe: Prevent triggering kretprobe from within kprobe_flush_task
> > 
> > Masami Hiramatsu (4):
> >       kprobes: Suppress the suspicious RCU warning on kprobes
> >       kprobes: Use non RCU traversal APIs on kprobe_tables if possible
> >       kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex
> >       kprobes: Remove redundant arch_disarm_kprobe() call
> > 
> > 
> >  arch/x86/kernel/kprobes/core.c |   16 ++--------
> >  include/linux/kprobes.h        |    6 +++-
> >  kernel/kprobes.c               |   61 +++++++++++++++++++++++++++++++---------
> >  3 files changed, 56 insertions(+), 27 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups
  2020-06-14 15:37   ` Jiri Olsa
@ 2020-06-16 19:01     ` Steven Rostedt
  2020-06-16 19:44       ` Jiri Olsa
  2020-06-17 14:40       ` Masami Hiramatsu
  0 siblings, 2 replies; 12+ messages in thread
From: Steven Rostedt @ 2020-06-16 19:01 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Masami Hiramatsu, Ingo Molnar, Gustavo A . R . Silva,
	Anders Roxell, paulmck, joel, Naveen N . Rao,
	Anil S Keshavamurthy, David Miller, Linux Kernel Mailing List,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN, Jiri Olsa

On Sun, 14 Jun 2020 17:37:28 +0200
Jiri Olsa <jolsa@redhat.com> wrote:

> On Wed, May 27, 2020 at 11:49:41PM +0900, Masami Hiramatsu wrote:
> > (Oops, I missed Jiri in loop.)
> > 
> > Hi Ingo,
> > 
> > Could you take this series?
> > These are not adding any feature, but fixing real bugs.  
> 
> Hi,
> I still can't see this being pulled in, did I miss it?
> 

I'm getting a pull request ready for -rc2. I can pull these in with that.

-- Steve

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups
  2020-06-16 19:01     ` Steven Rostedt
@ 2020-06-16 19:44       ` Jiri Olsa
  2020-06-17 14:40       ` Masami Hiramatsu
  1 sibling, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2020-06-16 19:44 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Ingo Molnar, Gustavo A . R . Silva,
	Anders Roxell, paulmck, joel, Naveen N . Rao,
	Anil S Keshavamurthy, David Miller, Linux Kernel Mailing List,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN, Jiri Olsa

On Tue, Jun 16, 2020 at 03:01:22PM -0400, Steven Rostedt wrote:
> On Sun, 14 Jun 2020 17:37:28 +0200
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > On Wed, May 27, 2020 at 11:49:41PM +0900, Masami Hiramatsu wrote:
> > > (Oops, I missed Jiri in loop.)
> > > 
> > > Hi Ingo,
> > > 
> > > Could you take this series?
> > > These are not adding any feature, but fixing real bugs.  
> > 
> > Hi,
> > I still can't see this being pulled in, did I miss it?
> > 
> 
> I'm getting a pull request ready for -rc2. I can pull these in with that.

that'd be awesome, thanks!

jirka


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups
  2020-06-16 19:01     ` Steven Rostedt
  2020-06-16 19:44       ` Jiri Olsa
@ 2020-06-17 14:40       ` Masami Hiramatsu
  1 sibling, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2020-06-17 14:40 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiri Olsa, Masami Hiramatsu, Ingo Molnar, Gustavo A . R . Silva,
	Anders Roxell, paulmck, joel, Naveen N . Rao,
	Anil S Keshavamurthy, David Miller, Linux Kernel Mailing List,
	Ingo Molnar, Peter Zijlstra, Ziqian SUN, Jiri Olsa

On Tue, 16 Jun 2020 15:01:22 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sun, 14 Jun 2020 17:37:28 +0200
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > On Wed, May 27, 2020 at 11:49:41PM +0900, Masami Hiramatsu wrote:
> > > (Oops, I missed Jiri in loop.)
> > > 
> > > Hi Ingo,
> > > 
> > > Could you take this series?
> > > These are not adding any feature, but fixing real bugs.  
> > 
> > Hi,
> > I still can't see this being pulled in, did I miss it?
> > 
> 
> I'm getting a pull request ready for -rc2. I can pull these in with that.

Great! Thank you so much!


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-06-17 14:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-12  8:02 [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
2020-05-12  8:02 ` [PATCH -tip V6 1/6] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
2020-05-12  8:02 ` [PATCH -tip V6 2/6] kprobes: Use non RCU traversal APIs on kprobe_tables if possible Masami Hiramatsu
2020-05-12  8:02 ` [PATCH -tip V6 3/6] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex Masami Hiramatsu
2020-05-12  8:03 ` [PATCH -tip V6 4/6] kprobes: Remove redundant arch_disarm_kprobe() call Masami Hiramatsu
2020-05-12  8:03 ` [PATCH -tip V6 5/6] kretprobe: Prevent triggering kretprobe from within kprobe_flush_task Masami Hiramatsu
2020-05-12  8:03 ` [PATCH -tip V6 6/6] kprobes: Replace zero-length array with flexible-array Masami Hiramatsu
2020-05-27 14:49 ` [PATCH -tip V6 0/6] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
2020-06-14 15:37   ` Jiri Olsa
2020-06-16 19:01     ` Steven Rostedt
2020-06-16 19:44       ` Jiri Olsa
2020-06-17 14:40       ` Masami Hiramatsu

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.