* [PATCH] perf tools: Fix crash with non-jited bpf progs
@ 2020-10-16 11:47 Tommi Rantala
2020-10-19 20:53 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Tommi Rantala @ 2020-10-16 11:47 UTC (permalink / raw)
To: linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim
Cc: Tommi Rantala
The addr in PERF_RECORD_KSYMBOL events for non-jited bpf progs points to
the bpf interpreter, ie. within kernel text section. When processing the
unregister event, this causes unexpected removal of vmlinux_map,
crashing perf later in cleanup:
# perf record -- timeout --signal=INT 2s /usr/share/bcc/tools/execsnoop
PCOMM PID PPID RET ARGS
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.208 MB perf.data (5155 samples) ]
perf: tools/include/linux/refcount.h:131: refcount_sub_and_test: Assertion `!(new > val)' failed.
Aborted (core dumped)
# perf script -D|grep KSYM
0 0xa40 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_f958f6eb72ef5af6
0 0xab0 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_8c42dee26e8cd4c2
0 0xb20 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_f958f6eb72ef5af6
108563691893 0x33d98 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3b0 len 0 type 1 flags 0x0 name bpf_prog_bc5697a410556fc2_syscall__execve
108568518458 0x34098 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3f0 len 0 type 1 flags 0x0 name bpf_prog_45e2203c2928704d_do_ret_sys_execve
109301967895 0x34830 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3b0 len 0 type 1 flags 0x1 name bpf_prog_bc5697a410556fc2_syscall__execve
109302007356 0x348b0 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3f0 len 0 type 1 flags 0x1 name bpf_prog_45e2203c2928704d_do_ret_sys_execve
perf: tools/include/linux/refcount.h:131: refcount_sub_and_test: Assertion `!(new > val)' failed.
Here the addresses match the bpf interpreter:
# grep -e ffffffffa9b6b530 -e ffffffffa9b6b3b0 -e ffffffffa9b6b3f0 /proc/kallsyms
ffffffffa9b6b3b0 t __bpf_prog_run224
ffffffffa9b6b3f0 t __bpf_prog_run192
ffffffffa9b6b530 t __bpf_prog_run32
Fix by not allowing vmlinux_map to be removed by PERF_RECORD_KSYMBOL
unregister event.
Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
tools/perf/util/machine.c | 11 ++++++++++-
tools/perf/util/symbol.c | 7 +++++++
tools/perf/util/symbol.h | 2 ++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 85587de027a5..d93d35463c61 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -786,11 +786,20 @@ static int machine__process_ksymbol_unregister(struct machine *machine,
union perf_event *event,
struct perf_sample *sample __maybe_unused)
{
+ struct symbol *sym;
struct map *map;
map = maps__find(&machine->kmaps, event->ksymbol.addr);
- if (map)
+ if (!map)
+ return 0;
+
+ if (map != machine->vmlinux_map)
maps__remove(&machine->kmaps, map);
+ else {
+ sym = dso__find_symbol(map->dso, map->map_ip(map, map->start));
+ if (sym)
+ dso__delete_symbol(map->dso, sym);
+ }
return 0;
}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5151a8c0b791..6bf8e74ea1d1 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -515,6 +515,13 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym)
}
}
+void dso__delete_symbol(struct dso *dso, struct symbol *sym)
+{
+ rb_erase_cached(&sym->rb_node, &dso->symbols);
+ symbol__delete(sym);
+ dso__reset_find_symbol_cache(dso);
+}
+
struct symbol *dso__find_symbol(struct dso *dso, u64 addr)
{
if (dso->last_find_result.addr != addr || dso->last_find_result.symbol == NULL) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 03e264a27cd3..60345691db09 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -130,6 +130,8 @@ int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map);
void dso__insert_symbol(struct dso *dso,
struct symbol *sym);
+void dso__delete_symbol(struct dso *dso,
+ struct symbol *sym);
struct symbol *dso__find_symbol(struct dso *dso, u64 addr);
struct symbol *dso__find_symbol_by_name(struct dso *dso, const char *name);
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf tools: Fix crash with non-jited bpf progs
2020-10-16 11:47 [PATCH] perf tools: Fix crash with non-jited bpf progs Tommi Rantala
@ 2020-10-19 20:53 ` Jiri Olsa
2020-10-20 12:11 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2020-10-19 20:53 UTC (permalink / raw)
To: Tommi Rantala
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Namhyung Kim
On Fri, Oct 16, 2020 at 02:47:18PM +0300, Tommi Rantala wrote:
> The addr in PERF_RECORD_KSYMBOL events for non-jited bpf progs points to
> the bpf interpreter, ie. within kernel text section. When processing the
> unregister event, this causes unexpected removal of vmlinux_map,
> crashing perf later in cleanup:
>
> # perf record -- timeout --signal=INT 2s /usr/share/bcc/tools/execsnoop
> PCOMM PID PPID RET ARGS
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.208 MB perf.data (5155 samples) ]
> perf: tools/include/linux/refcount.h:131: refcount_sub_and_test: Assertion `!(new > val)' failed.
> Aborted (core dumped)
>
> # perf script -D|grep KSYM
> 0 0xa40 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_f958f6eb72ef5af6
> 0 0xab0 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_8c42dee26e8cd4c2
> 0 0xb20 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_f958f6eb72ef5af6
> 108563691893 0x33d98 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3b0 len 0 type 1 flags 0x0 name bpf_prog_bc5697a410556fc2_syscall__execve
> 108568518458 0x34098 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3f0 len 0 type 1 flags 0x0 name bpf_prog_45e2203c2928704d_do_ret_sys_execve
> 109301967895 0x34830 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3b0 len 0 type 1 flags 0x1 name bpf_prog_bc5697a410556fc2_syscall__execve
> 109302007356 0x348b0 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3f0 len 0 type 1 flags 0x1 name bpf_prog_45e2203c2928704d_do_ret_sys_execve
> perf: tools/include/linux/refcount.h:131: refcount_sub_and_test: Assertion `!(new > val)' failed.
>
> Here the addresses match the bpf interpreter:
>
> # grep -e ffffffffa9b6b530 -e ffffffffa9b6b3b0 -e ffffffffa9b6b3f0 /proc/kallsyms
> ffffffffa9b6b3b0 t __bpf_prog_run224
> ffffffffa9b6b3f0 t __bpf_prog_run192
> ffffffffa9b6b530 t __bpf_prog_run32
>
> Fix by not allowing vmlinux_map to be removed by PERF_RECORD_KSYMBOL
> unregister event.
>
> Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
nice, I almost forgot about non jit mode by now ;-)
Acked/Tested-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
> ---
> tools/perf/util/machine.c | 11 ++++++++++-
> tools/perf/util/symbol.c | 7 +++++++
> tools/perf/util/symbol.h | 2 ++
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 85587de027a5..d93d35463c61 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -786,11 +786,20 @@ static int machine__process_ksymbol_unregister(struct machine *machine,
> union perf_event *event,
> struct perf_sample *sample __maybe_unused)
> {
> + struct symbol *sym;
> struct map *map;
>
> map = maps__find(&machine->kmaps, event->ksymbol.addr);
> - if (map)
> + if (!map)
> + return 0;
> +
> + if (map != machine->vmlinux_map)
> maps__remove(&machine->kmaps, map);
> + else {
> + sym = dso__find_symbol(map->dso, map->map_ip(map, map->start));
> + if (sym)
> + dso__delete_symbol(map->dso, sym);
> + }
>
> return 0;
> }
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 5151a8c0b791..6bf8e74ea1d1 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -515,6 +515,13 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym)
> }
> }
>
> +void dso__delete_symbol(struct dso *dso, struct symbol *sym)
> +{
> + rb_erase_cached(&sym->rb_node, &dso->symbols);
> + symbol__delete(sym);
> + dso__reset_find_symbol_cache(dso);
> +}
> +
> struct symbol *dso__find_symbol(struct dso *dso, u64 addr)
> {
> if (dso->last_find_result.addr != addr || dso->last_find_result.symbol == NULL) {
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 03e264a27cd3..60345691db09 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -130,6 +130,8 @@ int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map);
>
> void dso__insert_symbol(struct dso *dso,
> struct symbol *sym);
> +void dso__delete_symbol(struct dso *dso,
> + struct symbol *sym);
>
> struct symbol *dso__find_symbol(struct dso *dso, u64 addr);
> struct symbol *dso__find_symbol_by_name(struct dso *dso, const char *name);
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] perf tools: Fix crash with non-jited bpf progs
2020-10-19 20:53 ` Jiri Olsa
@ 2020-10-20 12:11 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-20 12:11 UTC (permalink / raw)
To: Jiri Olsa
Cc: Tommi Rantala, linux-kernel, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Namhyung Kim
Em Mon, Oct 19, 2020 at 10:53:43PM +0200, Jiri Olsa escreveu:
> On Fri, Oct 16, 2020 at 02:47:18PM +0300, Tommi Rantala wrote:
> > The addr in PERF_RECORD_KSYMBOL events for non-jited bpf progs points to
> > the bpf interpreter, ie. within kernel text section. When processing the
> > unregister event, this causes unexpected removal of vmlinux_map,
> > crashing perf later in cleanup:
> >
> > # perf record -- timeout --signal=INT 2s /usr/share/bcc/tools/execsnoop
> > PCOMM PID PPID RET ARGS
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.208 MB perf.data (5155 samples) ]
> > perf: tools/include/linux/refcount.h:131: refcount_sub_and_test: Assertion `!(new > val)' failed.
> > Aborted (core dumped)
> >
> > # perf script -D|grep KSYM
> > 0 0xa40 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_f958f6eb72ef5af6
> > 0 0xab0 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_8c42dee26e8cd4c2
> > 0 0xb20 [0x48]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b530 len 0 type 1 flags 0x0 name bpf_prog_f958f6eb72ef5af6
> > 108563691893 0x33d98 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3b0 len 0 type 1 flags 0x0 name bpf_prog_bc5697a410556fc2_syscall__execve
> > 108568518458 0x34098 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3f0 len 0 type 1 flags 0x0 name bpf_prog_45e2203c2928704d_do_ret_sys_execve
> > 109301967895 0x34830 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3b0 len 0 type 1 flags 0x1 name bpf_prog_bc5697a410556fc2_syscall__execve
> > 109302007356 0x348b0 [0x58]: PERF_RECORD_KSYMBOL addr ffffffffa9b6b3f0 len 0 type 1 flags 0x1 name bpf_prog_45e2203c2928704d_do_ret_sys_execve
> > perf: tools/include/linux/refcount.h:131: refcount_sub_and_test: Assertion `!(new > val)' failed.
> >
> > Here the addresses match the bpf interpreter:
> >
> > # grep -e ffffffffa9b6b530 -e ffffffffa9b6b3b0 -e ffffffffa9b6b3f0 /proc/kallsyms
> > ffffffffa9b6b3b0 t __bpf_prog_run224
> > ffffffffa9b6b3f0 t __bpf_prog_run192
> > ffffffffa9b6b530 t __bpf_prog_run32
> >
> > Fix by not allowing vmlinux_map to be removed by PERF_RECORD_KSYMBOL
> > unregister event.
> >
> > Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
>
> nice, I almost forgot about non jit mode by now ;-)
>
> Acked/Tested-by: Jiri Olsa <jolsa@redhat.com>
Thanks, applied.
- Arnaldo
> thanks,
> jirka
>
> > ---
> > tools/perf/util/machine.c | 11 ++++++++++-
> > tools/perf/util/symbol.c | 7 +++++++
> > tools/perf/util/symbol.h | 2 ++
> > 3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 85587de027a5..d93d35463c61 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -786,11 +786,20 @@ static int machine__process_ksymbol_unregister(struct machine *machine,
> > union perf_event *event,
> > struct perf_sample *sample __maybe_unused)
> > {
> > + struct symbol *sym;
> > struct map *map;
> >
> > map = maps__find(&machine->kmaps, event->ksymbol.addr);
> > - if (map)
> > + if (!map)
> > + return 0;
> > +
> > + if (map != machine->vmlinux_map)
> > maps__remove(&machine->kmaps, map);
> > + else {
> > + sym = dso__find_symbol(map->dso, map->map_ip(map, map->start));
> > + if (sym)
> > + dso__delete_symbol(map->dso, sym);
> > + }
> >
> > return 0;
> > }
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 5151a8c0b791..6bf8e74ea1d1 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -515,6 +515,13 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym)
> > }
> > }
> >
> > +void dso__delete_symbol(struct dso *dso, struct symbol *sym)
> > +{
> > + rb_erase_cached(&sym->rb_node, &dso->symbols);
> > + symbol__delete(sym);
> > + dso__reset_find_symbol_cache(dso);
> > +}
> > +
> > struct symbol *dso__find_symbol(struct dso *dso, u64 addr)
> > {
> > if (dso->last_find_result.addr != addr || dso->last_find_result.symbol == NULL) {
> > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > index 03e264a27cd3..60345691db09 100644
> > --- a/tools/perf/util/symbol.h
> > +++ b/tools/perf/util/symbol.h
> > @@ -130,6 +130,8 @@ int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map);
> >
> > void dso__insert_symbol(struct dso *dso,
> > struct symbol *sym);
> > +void dso__delete_symbol(struct dso *dso,
> > + struct symbol *sym);
> >
> > struct symbol *dso__find_symbol(struct dso *dso, u64 addr);
> > struct symbol *dso__find_symbol_by_name(struct dso *dso, const char *name);
> > --
> > 2.26.2
> >
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-20 12:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-16 11:47 [PATCH] perf tools: Fix crash with non-jited bpf progs Tommi Rantala
2020-10-19 20:53 ` Jiri Olsa
2020-10-20 12:11 ` Arnaldo Carvalho de Melo
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.