* [PATCH v2] kprobes: Fix kill kprobe which has been marked as gone
@ 2020-08-22 3:00 Muchun Song
2020-08-31 2:59 ` Muchun Song
0 siblings, 1 reply; 4+ messages in thread
From: Muchun Song @ 2020-08-22 3:00 UTC (permalink / raw)
To: naveen.n.rao, anil.s.keshavamurthy, davem, mhiramat, songliubraving
Cc: linux-kernel, Muchun Song, Chengming Zhou
If a kprobe is marked as gone, we should not kill it again. Otherwise,
we can disarm the kprobe more than once. In that case, the statistics
of kprobe_ftrace_enabled can unbalance which can lead to that kprobe
do not work.
Fixes: e8386a0cb22f ("kprobes: support probing module __exit function")
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
changelogs in v2:
1. Add a WARN_ON_ONCE in the kill_kprobe() to catch incorrect use of it.
2. Update 'Fixes' tag in the commmit log.
kernel/kprobes.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d36e2b017588..9348b0c36ae0 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2143,6 +2143,9 @@ static void kill_kprobe(struct kprobe *p)
lockdep_assert_held(&kprobe_mutex);
+ if (WARN_ON_ONCE(kprobe_gone(p)))
+ return;
+
p->flags |= KPROBE_FLAG_GONE;
if (kprobe_aggrprobe(p)) {
/*
@@ -2422,7 +2425,10 @@ 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(p, head, hlist)
+ hlist_for_each_entry(p, head, hlist) {
+ if (kprobe_gone(p))
+ continue;
+
if (within_module_init((unsigned long)p->addr, mod) ||
(checkcore &&
within_module_core((unsigned long)p->addr, mod))) {
@@ -2439,6 +2445,7 @@ static int kprobes_module_callback(struct notifier_block *nb,
*/
kill_kprobe(p);
}
+ }
}
if (val == MODULE_STATE_GOING)
remove_module_kprobe_blacklist(mod);
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] kprobes: Fix kill kprobe which has been marked as gone
2020-08-22 3:00 [PATCH v2] kprobes: Fix kill kprobe which has been marked as gone Muchun Song
@ 2020-08-31 2:59 ` Muchun Song
2020-09-02 3:05 ` Masami Hiramatsu
0 siblings, 1 reply; 4+ messages in thread
From: Muchun Song @ 2020-08-31 2:59 UTC (permalink / raw)
To: naveen.n.rao, anil.s.keshavamurthy, davem, Masami Hiramatsu,
songliubraving
Cc: LKML, Chengming Zhou, Steven Rostedt, Andrew Morton
Cc Andrew and Steven.
Any other comments or someone can add this to the queue for the
merge window? It's worth fixing it.
On Sat, Aug 22, 2020 at 11:01 AM Muchun Song <songmuchun@bytedance.com> wrote:
>
> If a kprobe is marked as gone, we should not kill it again. Otherwise,
> we can disarm the kprobe more than once. In that case, the statistics
> of kprobe_ftrace_enabled can unbalance which can lead to that kprobe
> do not work.
>
> Fixes: e8386a0cb22f ("kprobes: support probing module __exit function")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> changelogs in v2:
> 1. Add a WARN_ON_ONCE in the kill_kprobe() to catch incorrect use of it.
> 2. Update 'Fixes' tag in the commmit log.
>
> kernel/kprobes.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d36e2b017588..9348b0c36ae0 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2143,6 +2143,9 @@ static void kill_kprobe(struct kprobe *p)
>
> lockdep_assert_held(&kprobe_mutex);
>
> + if (WARN_ON_ONCE(kprobe_gone(p)))
> + return;
> +
> p->flags |= KPROBE_FLAG_GONE;
> if (kprobe_aggrprobe(p)) {
> /*
> @@ -2422,7 +2425,10 @@ 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(p, head, hlist)
> + hlist_for_each_entry(p, head, hlist) {
> + if (kprobe_gone(p))
> + continue;
> +
> if (within_module_init((unsigned long)p->addr, mod) ||
> (checkcore &&
> within_module_core((unsigned long)p->addr, mod))) {
> @@ -2439,6 +2445,7 @@ static int kprobes_module_callback(struct notifier_block *nb,
> */
> kill_kprobe(p);
> }
> + }
> }
> if (val == MODULE_STATE_GOING)
> remove_module_kprobe_blacklist(mod);
> --
> 2.11.0
>
--
Yours,
Muchun
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] kprobes: Fix kill kprobe which has been marked as gone
2020-08-31 2:59 ` Muchun Song
@ 2020-09-02 3:05 ` Masami Hiramatsu
2020-09-02 3:15 ` [External] " Muchun Song
0 siblings, 1 reply; 4+ messages in thread
From: Masami Hiramatsu @ 2020-09-02 3:05 UTC (permalink / raw)
To: Muchun Song, Ingo Molnar
Cc: naveen.n.rao, anil.s.keshavamurthy, davem, songliubraving, LKML,
Chengming Zhou, Steven Rostedt, Andrew Morton
Hi Ingo,
Could you merge this fix to -tip?
I can resend it with other kprobes fixes.
Hi Muchun,
We also need;
Cc: stable@vger.kernel.org
for bugfix so that the patch can be backported correctly after merged to upstream.
Thank you,
On Mon, 31 Aug 2020 10:59:19 +0800
Muchun Song <songmuchun@bytedance.com> wrote:
> Cc Andrew and Steven.
>
> Any other comments or someone can add this to the queue for the
> merge window? It's worth fixing it.
>
> On Sat, Aug 22, 2020 at 11:01 AM Muchun Song <songmuchun@bytedance.com> wrote:
> >
> > If a kprobe is marked as gone, we should not kill it again. Otherwise,
> > we can disarm the kprobe more than once. In that case, the statistics
> > of kprobe_ftrace_enabled can unbalance which can lead to that kprobe
> > do not work.
> >
> > Fixes: e8386a0cb22f ("kprobes: support probing module __exit function")
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
> > Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > changelogs in v2:
> > 1. Add a WARN_ON_ONCE in the kill_kprobe() to catch incorrect use of it.
> > 2. Update 'Fixes' tag in the commmit log.
> >
> > kernel/kprobes.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index d36e2b017588..9348b0c36ae0 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -2143,6 +2143,9 @@ static void kill_kprobe(struct kprobe *p)
> >
> > lockdep_assert_held(&kprobe_mutex);
> >
> > + if (WARN_ON_ONCE(kprobe_gone(p)))
> > + return;
> > +
> > p->flags |= KPROBE_FLAG_GONE;
> > if (kprobe_aggrprobe(p)) {
> > /*
> > @@ -2422,7 +2425,10 @@ 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(p, head, hlist)
> > + hlist_for_each_entry(p, head, hlist) {
> > + if (kprobe_gone(p))
> > + continue;
> > +
> > if (within_module_init((unsigned long)p->addr, mod) ||
> > (checkcore &&
> > within_module_core((unsigned long)p->addr, mod))) {
> > @@ -2439,6 +2445,7 @@ static int kprobes_module_callback(struct notifier_block *nb,
> > */
> > kill_kprobe(p);
> > }
> > + }
> > }
> > if (val == MODULE_STATE_GOING)
> > remove_module_kprobe_blacklist(mod);
> > --
> > 2.11.0
> >
>
>
> --
> Yours,
> Muchun
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [External] Re: [PATCH v2] kprobes: Fix kill kprobe which has been marked as gone
2020-09-02 3:05 ` Masami Hiramatsu
@ 2020-09-02 3:15 ` Muchun Song
0 siblings, 0 replies; 4+ messages in thread
From: Muchun Song @ 2020-09-02 3:15 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Ingo Molnar, naveen.n.rao, anil.s.keshavamurthy, davem,
songliubraving, LKML, Chengming Zhou, Steven Rostedt,
Andrew Morton
Hi Masami,
On Wed, Sep 2, 2020 at 11:05 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi Ingo,
>
> Could you merge this fix to -tip?
This patch has been merged into Andrew's mm tree.
>
> I can resend it with other kprobes fixes.
>
> Hi Muchun,
>
> We also need;
>
> Cc: stable@vger.kernel.org
>
> for bugfix so that the patch can be backported correctly after merged to upstream.
Yeah, I got it. Thanks.
>
> Thank you,
>
> On Mon, 31 Aug 2020 10:59:19 +0800
> Muchun Song <songmuchun@bytedance.com> wrote:
>
> > Cc Andrew and Steven.
> >
> > Any other comments or someone can add this to the queue for the
> > merge window? It's worth fixing it.
> >
> > On Sat, Aug 22, 2020 at 11:01 AM Muchun Song <songmuchun@bytedance.com> wrote:
> > >
> > > If a kprobe is marked as gone, we should not kill it again. Otherwise,
> > > we can disarm the kprobe more than once. In that case, the statistics
> > > of kprobe_ftrace_enabled can unbalance which can lead to that kprobe
> > > do not work.
> > >
> > > Fixes: e8386a0cb22f ("kprobes: support probing module __exit function")
> > > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > > Co-developed-by: Chengming Zhou <zhouchengming@bytedance.com>
> > > Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
> > > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > > changelogs in v2:
> > > 1. Add a WARN_ON_ONCE in the kill_kprobe() to catch incorrect use of it.
> > > 2. Update 'Fixes' tag in the commmit log.
> > >
> > > kernel/kprobes.c | 9 ++++++++-
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > > index d36e2b017588..9348b0c36ae0 100644
> > > --- a/kernel/kprobes.c
> > > +++ b/kernel/kprobes.c
> > > @@ -2143,6 +2143,9 @@ static void kill_kprobe(struct kprobe *p)
> > >
> > > lockdep_assert_held(&kprobe_mutex);
> > >
> > > + if (WARN_ON_ONCE(kprobe_gone(p)))
> > > + return;
> > > +
> > > p->flags |= KPROBE_FLAG_GONE;
> > > if (kprobe_aggrprobe(p)) {
> > > /*
> > > @@ -2422,7 +2425,10 @@ 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(p, head, hlist)
> > > + hlist_for_each_entry(p, head, hlist) {
> > > + if (kprobe_gone(p))
> > > + continue;
> > > +
> > > if (within_module_init((unsigned long)p->addr, mod) ||
> > > (checkcore &&
> > > within_module_core((unsigned long)p->addr, mod))) {
> > > @@ -2439,6 +2445,7 @@ static int kprobes_module_callback(struct notifier_block *nb,
> > > */
> > > kill_kprobe(p);
> > > }
> > > + }
> > > }
> > > if (val == MODULE_STATE_GOING)
> > > remove_module_kprobe_blacklist(mod);
> > > --
> > > 2.11.0
> > >
> >
> >
> > --
> > Yours,
> > Muchun
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
--
Yours,
Muchun
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-02 3:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-22 3:00 [PATCH v2] kprobes: Fix kill kprobe which has been marked as gone Muchun Song
2020-08-31 2:59 ` Muchun Song
2020-09-02 3:05 ` Masami Hiramatsu
2020-09-02 3:15 ` [External] " Muchun Song
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).