All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -tip] kprobes: Fix an inverted result check for reusing optimized probe
@ 2019-04-15  6:01 Masami Hiramatsu
  2019-04-16  7:43 ` [tip:perf/urgent] kprobes: Fix error check when reusing optimized probes tip-bot for Masami Hiramatsu
  0 siblings, 1 reply; 2+ messages in thread
From: Masami Hiramatsu @ 2019-04-15  6:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Anil S Keshavamurthy, David S . Miller, Linus Torvalds,
	Naveen N . Rao, Peter Zijlstra, Thomas Gleixner,
	Masami Hiramatsu, stable, linux-kernel

Fix an inverted result check for reusing unused kprobe correctly.
This has been introduced by commit 819319fc9346 ("kprobes: Return
error if we fail to reuse kprobe instead of BUG_ON()"), which
missed to handle the return value of kprobe_optready() as
error-value. In reality, the kprobe_optready() returns a bool
result, so "true" case must be passed instead of 0.

This causes some errors on kprobe boot-time selftests on arm.

[    4.563544] Beginning kprobe tests...
[    4.563648] Probe ARM code
[    4.563733]     kprobe
[    4.564700]     kretprobe
[    4.565538] ARM instruction simulation
[    4.565671]     Check decoding tables
[    4.565883]     Run test cases
[    5.070700] FAIL: test_case_handler not run
[    5.070938] FAIL: Test andge	r10, r11, r14, asr r7
[    5.071118] FAIL: Scenario 11
...
[   74.174729] FAIL: Scenario 7
[   74.211776] Total instruction simulation tests=1631, pass=1433 fail=198
[   74.212168] kprobe tests failed

This can happen if an optimized probe is unregistered and next
kprobe is registered on same address until the previous probe
is not reclaimed.

If this happens, a hidden aggregated probe may be kept in memory,
and no new kprobe can probe same address. Also, in that case
register_kprobe() will return "1" instead of minus error value,
which can mislead caller logic.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 819319fc9346 ("kprobes: Return error if we fail to reuse kprobe instead of BUG_ON()")
Cc: stable@vger.kernel.org # v5.0+
---
 kernel/kprobes.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index c83e54727131..b1ea30a5540e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -709,7 +709,6 @@ static void unoptimize_kprobe(struct kprobe *p, bool force)
 static int reuse_unused_kprobe(struct kprobe *ap)
 {
 	struct optimized_kprobe *op;
-	int ret;
 
 	/*
 	 * Unused kprobe MUST be on the way of delayed unoptimizing (means
@@ -720,9 +719,8 @@ static int reuse_unused_kprobe(struct kprobe *ap)
 	/* Enable the probe again */
 	ap->flags &= ~KPROBE_FLAG_DISABLED;
 	/* Optimize it again (remove from op->list) */
-	ret = kprobe_optready(ap);
-	if (ret)
-		return ret;
+	if (!kprobe_optready(ap))
+		return -EINVAL;
 
 	optimize_kprobe(ap);
 	return 0;


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

* [tip:perf/urgent] kprobes: Fix error check when reusing optimized probes
  2019-04-15  6:01 [PATCH -tip] kprobes: Fix an inverted result check for reusing optimized probe Masami Hiramatsu
@ 2019-04-16  7:43 ` tip-bot for Masami Hiramatsu
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2019-04-16  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mhiramat, mingo, anil.s.keshavamurthy, peterz, hpa, davem,
	naveen.n.rao, tglx, linux-kernel, torvalds

Commit-ID:  5f843ed415581cfad4ef8fefe31c138a8346ca8a
Gitweb:     https://git.kernel.org/tip/5f843ed415581cfad4ef8fefe31c138a8346ca8a
Author:     Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate: Mon, 15 Apr 2019 15:01:25 +0900
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 16 Apr 2019 09:38:16 +0200

kprobes: Fix error check when reusing optimized probes

The following commit introduced a bug in one of our error paths:

  819319fc9346 ("kprobes: Return error if we fail to reuse kprobe instead of BUG_ON()")

it missed to handle the return value of kprobe_optready() as
error-value. In reality, the kprobe_optready() returns a bool
result, so "true" case must be passed instead of 0.

This causes some errors on kprobe boot-time selftests on ARM:

 [   ] Beginning kprobe tests...
 [   ] Probe ARM code
 [   ]     kprobe
 [   ]     kretprobe
 [   ] ARM instruction simulation
 [   ]     Check decoding tables
 [   ]     Run test cases
 [   ] FAIL: test_case_handler not run
 [   ] FAIL: Test andge	r10, r11, r14, asr r7
 [   ] FAIL: Scenario 11
 ...
 [   ] FAIL: Scenario 7
 [   ] Total instruction simulation tests=1631, pass=1433 fail=198
 [   ] kprobe tests failed

This can happen if an optimized probe is unregistered and next
kprobe is registered on same address until the previous probe
is not reclaimed.

If this happens, a hidden aggregated probe may be kept in memory,
and no new kprobe can probe same address. Also, in that case
register_kprobe() will return "1" instead of minus error value,
which can mislead caller logic.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Naveen N . Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org # v5.0+
Fixes: 819319fc9346 ("kprobes: Return error if we fail to reuse kprobe instead of BUG_ON()")
Link: http://lkml.kernel.org/r/155530808559.32517.539898325433642204.stgit@devnote2
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/kprobes.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index c83e54727131..b1ea30a5540e 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -709,7 +709,6 @@ static void unoptimize_kprobe(struct kprobe *p, bool force)
 static int reuse_unused_kprobe(struct kprobe *ap)
 {
 	struct optimized_kprobe *op;
-	int ret;
 
 	/*
 	 * Unused kprobe MUST be on the way of delayed unoptimizing (means
@@ -720,9 +719,8 @@ static int reuse_unused_kprobe(struct kprobe *ap)
 	/* Enable the probe again */
 	ap->flags &= ~KPROBE_FLAG_DISABLED;
 	/* Optimize it again (remove from op->list) */
-	ret = kprobe_optready(ap);
-	if (ret)
-		return ret;
+	if (!kprobe_optready(ap))
+		return -EINVAL;
 
 	optimize_kprobe(ap);
 	return 0;

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

end of thread, other threads:[~2019-04-16  7:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15  6:01 [PATCH -tip] kprobes: Fix an inverted result check for reusing optimized probe Masami Hiramatsu
2019-04-16  7:43 ` [tip:perf/urgent] kprobes: Fix error check when reusing optimized probes tip-bot for 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.