RE: [PATCH] perf probe: Clear probe_trace_event when add_probe_trace_event() fails

[åæéå / HIRAMATUïMASAMI]

>From: Wang Nan [mailto:wangnan0@xxxxxxxxxx]
>
>When probe with glob, error in add_probe_trace_event() won't be passed
>to debuginfo__find_trace_events() because it whould be modified by
>probe_point_search_cb(). It causes segfault if perf failed to find
>argument for one probing point matched by the glob. For example:
>
> # ./perf probe -v -n 'SyS_dup? oldfd'
> probe-definition(0): SyS_dup? oldfd
> symbol:SyS_dup? file:(null) line:0 offset:0 return:0 lazy:(null)
> parsing arg: oldfd into oldfd
> 1 arguments
> Looking at the vmlinux_path (7 entries long)
> Using /lib/modules/4.3.0-rc4+/build/vmlinux for symbols
> Open Debuginfo file: /lib/modules/4.3.0-rc4+/build/vmlinux
> Try to find probe point from debuginfo.
> Matched function: SyS_dup3
> found inline addr: 0xffffffff812095c0
> Probe point found: SyS_dup3+0
> Searching 'oldfd' variable in context.
> Converting variable oldfd into trace event.
> oldfd type is long int.
> found inline addr: 0xffffffff812096d4
> Probe point found: SyS_dup2+36
> Searching 'oldfd' variable in context.
> Failed to find 'oldfd' in this function.
> Matched function: SyS_dup3
> Probe point found: SyS_dup3+0
> Searching 'oldfd' variable in context.
> Converting variable oldfd into trace event.
> oldfd type is long int.
> Matched function: SyS_dup2
> Probe point found: SyS_dup2+0
> Searching 'oldfd' variable in context.
> Converting variable oldfd into trace event.
> oldfd type is long int.
> Found 4 probe_trace_events.
> Opening /sys/kernel/debug/tracing//kprobe_events write=1
> Writing event: p:probe/SyS_dup3 _text+2135488 oldfd=%di:s64
> Segmentation fault (core dumped)
>
>This patch ensures add_probe_trace_event() not touch tf->ntevs and
>tf->tevs if it returns failure.
>
>Here is testing result:
>
> # perf probe  'SyS_dup? oldfd'
> Failed to find 'oldfd' in this function.
> Added new events:
>   probe:SyS_dup3       (on SyS_dup? with oldfd)
>   probe:SyS_dup3_1     (on SyS_dup? with oldfd)
>   probe:SyS_dup2       (on SyS_dup? with oldfd)
>
> You can now use it in all perf tools, such as:
>
>	perf record -e probe:SyS_dup2 -aR sleep 1

Good catch!

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>

Thanks!

>
>Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
>Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>Cc: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
>Cc: Zefan Li <lizefan@xxxxxxxxxx>
>Cc: pi3orama@xxxxxxx
>---
> tools/perf/util/probe-finder.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>index 63993d7..05012bb 100644
>--- a/tools/perf/util/probe-finder.c
>+++ b/tools/perf/util/probe-finder.c
>@@ -1183,7 +1183,7 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
> 			container_of(pf, struct trace_event_finder, pf);
> 	struct perf_probe_point *pp = &pf->pev->point;
> 	struct probe_trace_event *tev;
>-	struct perf_probe_arg *args;
>+	struct perf_probe_arg *args = NULL;
> 	int ret, i;
>
> 	/* Check number of tevs */
>@@ -1198,19 +1198,23 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
> 	ret = convert_to_trace_point(&pf->sp_die, tf->mod, pf->addr,
> 				     pp->retprobe, pp->function, &tev->point);
> 	if (ret < 0)
>-		return ret;
>+		goto end;
>
> 	tev->point.realname = strdup(dwarf_diename(sc_die));
>-	if (!tev->point.realname)
>-		return -ENOMEM;
>+	if (!tev->point.realname) {
>+		ret = -ENOMEM;
>+		goto end;
>+	}
>
> 	pr_debug("Probe point found: %s+%lu\n", tev->point.symbol,
> 		 tev->point.offset);
>
> 	/* Expand special probe argument if exist */
> 	args = zalloc(sizeof(struct perf_probe_arg) * MAX_PROBE_ARGS);
>-	if (args == NULL)
>-		return -ENOMEM;
>+	if (args == NULL) {
>+		ret = -ENOMEM;
>+		goto end;
>+	}
>
> 	ret = expand_probe_args(sc_die, pf, args);
> 	if (ret < 0)
>@@ -1234,6 +1238,10 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
> 	}
>
> end:
>+	if (ret) {
>+		clear_probe_trace_event(tev);
>+		tf->ntevs--;
>+	}
> 	free(args);
> 	return ret;
> }
>--
>1.8.3.4