* [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups
@ 2020-04-02 13:40 Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 1/4] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-04-02 13:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: Anders Roxell, paulmck, joel, Naveen N . Rao,
Anil S Keshavamurthy, David Miller, Masami Hiramatsu,
Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar
Hi,
Here is the 5th version of the series for kprobes.
I just updated the series on the latest tip/master branch and
add Fixes and Cc tag to [3/4].
Thank you,
---
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
kernel/kprobes.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH -tip V5 1/4] kprobes: Suppress the suspicious RCU warning on kprobes
2020-04-02 13:40 [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
@ 2020-04-02 13:40 ` Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 2/4] kprobes: Use non RCU traversal APIs on kprobe_tables if possible Masami Hiramatsu
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-04-02 13:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: Anders Roxell, paulmck, joel, Naveen N . Rao,
Anil S Keshavamurthy, David Miller, Masami Hiramatsu,
Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar
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] 6+ messages in thread
* [PATCH -tip V5 2/4] kprobes: Use non RCU traversal APIs on kprobe_tables if possible
2020-04-02 13:40 [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 1/4] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
@ 2020-04-02 13:40 ` Masami Hiramatsu
2020-04-02 13:41 ` [PATCH -tip V5 3/4] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex Masami Hiramatsu
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-04-02 13:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: Anders Roxell, paulmck, joel, Naveen N . Rao,
Anil S Keshavamurthy, David Miller, Masami Hiramatsu,
Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar
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] 6+ messages in thread
* [PATCH -tip V5 3/4] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex
2020-04-02 13:40 [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 1/4] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 2/4] kprobes: Use non RCU traversal APIs on kprobe_tables if possible Masami Hiramatsu
@ 2020-04-02 13:41 ` Masami Hiramatsu
2020-04-02 13:41 ` [PATCH -tip V5 4/4] kprobes: Remove redundant arch_disarm_kprobe() call Masami Hiramatsu
2020-04-09 14:33 ` [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
4 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-04-02 13:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: Anders Roxell, paulmck, joel, Naveen N . Rao,
Anil S Keshavamurthy, David Miller, Masami Hiramatsu,
Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar
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] 6+ messages in thread
* [PATCH -tip V5 4/4] kprobes: Remove redundant arch_disarm_kprobe() call
2020-04-02 13:40 [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
` (2 preceding siblings ...)
2020-04-02 13:41 ` [PATCH -tip V5 3/4] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex Masami Hiramatsu
@ 2020-04-02 13:41 ` Masami Hiramatsu
2020-04-09 14:33 ` [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
4 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-04-02 13:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: Anders Roxell, paulmck, joel, Naveen N . Rao,
Anil S Keshavamurthy, David Miller, Masami Hiramatsu,
Linux Kernel Mailing List, Steven Rostedt, Ingo Molnar
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] 6+ messages in thread
* Re: [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups
2020-04-02 13:40 [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
` (3 preceding siblings ...)
2020-04-02 13:41 ` [PATCH -tip V5 4/4] kprobes: Remove redundant arch_disarm_kprobe() call Masami Hiramatsu
@ 2020-04-09 14:33 ` Masami Hiramatsu
4 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-04-09 14:33 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Ingo Molnar, Anders Roxell, paulmck, joel, Naveen N . Rao,
Anil S Keshavamurthy, David Miller, Linux Kernel Mailing List,
Steven Rostedt, Ingo Molnar
Hi Ingo,
Could you pick this series or give me your comment?
Thank you,
On Thu, 2 Apr 2020 22:40:31 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is the 5th version of the series for kprobes.
> I just updated the series on the latest tip/master branch and
> add Fixes and Cc tag to [3/4].
>
> Thank you,
>
> ---
>
> 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
>
>
> kernel/kprobes.c | 37 ++++++++++++++++++++++++-------------
> 1 file changed, 24 insertions(+), 13 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-09 14:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-02 13:40 [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 1/4] kprobes: Suppress the suspicious RCU warning on kprobes Masami Hiramatsu
2020-04-02 13:40 ` [PATCH -tip V5 2/4] kprobes: Use non RCU traversal APIs on kprobe_tables if possible Masami Hiramatsu
2020-04-02 13:41 ` [PATCH -tip V5 3/4] kprobes: Fix to protect kick_kprobe_optimizer() by kprobe_mutex Masami Hiramatsu
2020-04-02 13:41 ` [PATCH -tip V5 4/4] kprobes: Remove redundant arch_disarm_kprobe() call Masami Hiramatsu
2020-04-09 14:33 ` [PATCH -tip V5 0/4] kprobes: Fixes mutex, rcu-list warnings and cleanups Masami Hiramatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).