* [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well
@ 2023-10-27 23:31 Andrii Nakryiko
2023-10-27 23:37 ` Song Liu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2023-10-27 23:31 UTC (permalink / raw)
To: linux-trace-kernel, mhiramat
Cc: bpf, Andrii Nakryiko, Francis Laniel, stable, Steven Rostedt
Recent changes to count number of matching symbols when creating
a kprobe event failed to take into account kernel modules. As such, it
breaks kprobes on kernel module symbols, by assuming there is no match.
Fix this my calling module_kallsyms_on_each_symbol() in addition to
kallsyms_on_each_match_symbol() to perform a proper counting.
Cc: Francis Laniel <flaniel@linux.microsoft.com>
Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/trace/trace_kprobe.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index effcaede4759..1efb27f35963 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -714,14 +714,30 @@ static int count_symbols(void *data, unsigned long unused)
return 0;
}
+struct sym_count_ctx {
+ unsigned int count;
+ const char *name;
+};
+
+static int count_mod_symbols(void *data, const char *name, unsigned long unused)
+{
+ struct sym_count_ctx *ctx = data;
+
+ if (strcmp(name, ctx->name) == 0)
+ ctx->count++;
+
+ return 0;
+}
+
static unsigned int number_of_same_symbols(char *func_name)
{
- unsigned int count;
+ struct sym_count_ctx ctx = { .count = 0, .name = func_name };
+
+ kallsyms_on_each_match_symbol(count_symbols, func_name, &ctx.count);
- count = 0;
- kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
+ module_kallsyms_on_each_symbol(NULL, count_mod_symbols, &ctx);
- return count;
+ return ctx.count;
}
static int __trace_kprobe_create(int argc, const char *argv[])
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well
2023-10-27 23:31 [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well Andrii Nakryiko
@ 2023-10-27 23:37 ` Song Liu
2023-10-28 0:50 ` Masami Hiramatsu
2023-10-28 1:41 ` Masami Hiramatsu
2023-10-29 17:09 ` Francis Laniel
2 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2023-10-27 23:37 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: linux-trace-kernel, mhiramat, bpf, Francis Laniel, stable,
Steven Rostedt
On Fri, Oct 27, 2023 at 4:31 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Recent changes to count number of matching symbols when creating
> a kprobe event failed to take into account kernel modules. As such, it
> breaks kprobes on kernel module symbols, by assuming there is no match.
>
> Fix this my calling module_kallsyms_on_each_symbol() in addition to
> kallsyms_on_each_match_symbol() to perform a proper counting.
>
> Cc: Francis Laniel <flaniel@linux.microsoft.com>
> Cc: stable@vger.kernel.org
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well
2023-10-27 23:37 ` Song Liu
@ 2023-10-28 0:50 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2023-10-28 0:50 UTC (permalink / raw)
To: Song Liu
Cc: Andrii Nakryiko, linux-trace-kernel, mhiramat, bpf,
Francis Laniel, stable, Steven Rostedt
On Fri, 27 Oct 2023 16:37:29 -0700
Song Liu <song@kernel.org> wrote:
> On Fri, Oct 27, 2023 at 4:31 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > Recent changes to count number of matching symbols when creating
> > a kprobe event failed to take into account kernel modules. As such, it
> > breaks kprobes on kernel module symbols, by assuming there is no match.
> >
> > Fix this my calling module_kallsyms_on_each_symbol() in addition to
> > kallsyms_on_each_match_symbol() to perform a proper counting.
> >
> > Cc: Francis Laniel <flaniel@linux.microsoft.com>
> > Cc: stable@vger.kernel.org
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> Acked-by: Song Liu <song@kernel.org>
Good catch! Thanks!
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well
2023-10-27 23:31 [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well Andrii Nakryiko
2023-10-27 23:37 ` Song Liu
@ 2023-10-28 1:41 ` Masami Hiramatsu
2023-10-28 3:18 ` Masami Hiramatsu
2023-10-29 17:09 ` Francis Laniel
2 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2023-10-28 1:41 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: linux-trace-kernel, bpf, Francis Laniel, stable, Steven Rostedt
Hi,
On Fri, 27 Oct 2023 16:31:26 -0700
Andrii Nakryiko <andrii@kernel.org> wrote:
> Recent changes to count number of matching symbols when creating
> a kprobe event failed to take into account kernel modules. As such, it
> breaks kprobes on kernel module symbols, by assuming there is no match.
>
> Fix this my calling module_kallsyms_on_each_symbol() in addition to
> kallsyms_on_each_match_symbol() to perform a proper counting.
>
> Cc: Francis Laniel <flaniel@linux.microsoft.com>
> Cc: stable@vger.kernel.org
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
This fixes "BPF" kprobe event, but not ftrace kprobe events.
ftrace kprobe events only checks this if it is in the vmlinux not
modules and that's why my selftest passed the original one.
Hmm, I need another enhancement like this for the events on offline
modules.
Thank you,
> ---
> kernel/trace/trace_kprobe.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index effcaede4759..1efb27f35963 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -714,14 +714,30 @@ static int count_symbols(void *data, unsigned long unused)
> return 0;
> }
>
> +struct sym_count_ctx {
> + unsigned int count;
> + const char *name;
> +};
> +
> +static int count_mod_symbols(void *data, const char *name, unsigned long unused)
> +{
> + struct sym_count_ctx *ctx = data;
> +
> + if (strcmp(name, ctx->name) == 0)
> + ctx->count++;
> +
> + return 0;
> +}
> +
> static unsigned int number_of_same_symbols(char *func_name)
> {
> - unsigned int count;
> + struct sym_count_ctx ctx = { .count = 0, .name = func_name };
> +
> + kallsyms_on_each_match_symbol(count_symbols, func_name, &ctx.count);
>
> - count = 0;
> - kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> + module_kallsyms_on_each_symbol(NULL, count_mod_symbols, &ctx);
>
> - return count;
> + return ctx.count;
> }
>
> static int __trace_kprobe_create(int argc, const char *argv[])
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well
2023-10-28 1:41 ` Masami Hiramatsu
@ 2023-10-28 3:18 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2023-10-28 3:18 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Andrii Nakryiko, linux-trace-kernel, bpf, Francis Laniel, stable,
Steven Rostedt
On Sat, 28 Oct 2023 10:41:44 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Hi,
>
> On Fri, 27 Oct 2023 16:31:26 -0700
> Andrii Nakryiko <andrii@kernel.org> wrote:
>
> > Recent changes to count number of matching symbols when creating
> > a kprobe event failed to take into account kernel modules. As such, it
> > breaks kprobes on kernel module symbols, by assuming there is no match.
> >
> > Fix this my calling module_kallsyms_on_each_symbol() in addition to
> > kallsyms_on_each_match_symbol() to perform a proper counting.
> >
> > Cc: Francis Laniel <flaniel@linux.microsoft.com>
> > Cc: stable@vger.kernel.org
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> This fixes "BPF" kprobe event, but not ftrace kprobe events.
> ftrace kprobe events only checks this if it is in the vmlinux not
> modules and that's why my selftest passed the original one.
Ah, my bad. ftrace kprobe event should accept the target symbol without
module name. Yes, in that case it missed the count.
> Hmm, I need another enhancement like this for the events on offline
> modules.
Also, I need to add another test case update without module name.
(this one should be another fix)
Thank you,
>
> Thank you,
>
> > ---
> > kernel/trace/trace_kprobe.c | 24 ++++++++++++++++++++----
> > 1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index effcaede4759..1efb27f35963 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -714,14 +714,30 @@ static int count_symbols(void *data, unsigned long unused)
> > return 0;
> > }
> >
> > +struct sym_count_ctx {
> > + unsigned int count;
> > + const char *name;
> > +};
> > +
> > +static int count_mod_symbols(void *data, const char *name, unsigned long unused)
> > +{
> > + struct sym_count_ctx *ctx = data;
> > +
> > + if (strcmp(name, ctx->name) == 0)
> > + ctx->count++;
> > +
> > + return 0;
> > +}
> > +
> > static unsigned int number_of_same_symbols(char *func_name)
> > {
> > - unsigned int count;
> > + struct sym_count_ctx ctx = { .count = 0, .name = func_name };
> > +
> > + kallsyms_on_each_match_symbol(count_symbols, func_name, &ctx.count);
> >
> > - count = 0;
> > - kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> > + module_kallsyms_on_each_symbol(NULL, count_mod_symbols, &ctx);
> >
> > - return count;
> > + return ctx.count;
> > }
> >
> > static int __trace_kprobe_create(int argc, const char *argv[])
> > --
> > 2.34.1
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well
2023-10-27 23:31 [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well Andrii Nakryiko
2023-10-27 23:37 ` Song Liu
2023-10-28 1:41 ` Masami Hiramatsu
@ 2023-10-29 17:09 ` Francis Laniel
2 siblings, 0 replies; 6+ messages in thread
From: Francis Laniel @ 2023-10-29 17:09 UTC (permalink / raw)
To: linux-trace-kernel, mhiramat, Andrii Nakryiko
Cc: bpf, Andrii Nakryiko, stable, Steven Rostedt
Hi!
Le samedi 28 octobre 2023, 02:31:26 EET Andrii Nakryiko a écrit :
> Recent changes to count number of matching symbols when creating
> a kprobe event failed to take into account kernel modules. As such, it
> breaks kprobes on kernel module symbols, by assuming there is no match.
Ah... Sorry about this...
Thank you for the fix, I will handle the patch for older kernels in case there
are troubles applying it.
> Fix this my calling module_kallsyms_on_each_symbol() in addition to
> kallsyms_on_each_match_symbol() to perform a proper counting.
>
> Cc: Francis Laniel <flaniel@linux.microsoft.com>
> Cc: stable@vger.kernel.org
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func
> matches several symbols") Signed-off-by: Andrii Nakryiko
> <andrii@kernel.org>
> ---
> kernel/trace/trace_kprobe.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index effcaede4759..1efb27f35963 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -714,14 +714,30 @@ static int count_symbols(void *data, unsigned long
> unused) return 0;
> }
>
> +struct sym_count_ctx {
> + unsigned int count;
> + const char *name;
> +};
> +
> +static int count_mod_symbols(void *data, const char *name, unsigned long
> unused) +{
> + struct sym_count_ctx *ctx = data;
> +
> + if (strcmp(name, ctx->name) == 0)
> + ctx->count++;
> +
> + return 0;
> +}
> +
> static unsigned int number_of_same_symbols(char *func_name)
> {
> - unsigned int count;
> + struct sym_count_ctx ctx = { .count = 0, .name = func_name };
> +
> + kallsyms_on_each_match_symbol(count_symbols, func_name, &ctx.count);
>
> - count = 0;
> - kallsyms_on_each_match_symbol(count_symbols, func_name, &count);
> + module_kallsyms_on_each_symbol(NULL, count_mod_symbols, &ctx);
>
> - return count;
> + return ctx.count;
> }
>
> static int __trace_kprobe_create(int argc, const char *argv[])
Best regards.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-29 17:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-27 23:31 [PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well Andrii Nakryiko
2023-10-27 23:37 ` Song Liu
2023-10-28 0:50 ` Masami Hiramatsu
2023-10-28 1:41 ` Masami Hiramatsu
2023-10-28 3:18 ` Masami Hiramatsu
2023-10-29 17:09 ` Francis Laniel
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).