| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 29 May 2015 10:34:00 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: jolsa@...hat.com, masami.hiramatsu.pt@...achi.com,
namhyung@...nel.org, lizefan@...wei.com,
linux-kernel@...r.kernel.org, pi3orama@....com
Subject: Re: [PATCH] perf probe: Fix segfault when glob matching function
without debuginfo
Em Fri, May 29, 2015 at 09:45:47AM +0000, Wang Nan escreveu:
> Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
> glob wildcards for function name") introduces segfault problems when
> debuginfo is not available:
>
> # perf probe 'sys_w*'
> Added new events:
> Segmentation fault
>
> The first problem resides in find_probe_trace_events_from_map(). In
> that function, find_probe_functions() is called to match each symbol
> against glob to find the number of matching functions, but still use
> map__for_each_symbol_by_name() to find 'struct symbol' for matching
> functions. Unfortunately, map__for_each_symbol_by_name() does
> exact matching by searching in an rbtree. It doesn't know glob
> matching, and not easy for it to support it because it use rbtree based
> binary search, but we are unable to ensure all names matched by the
> glob (any glob passed by user) reside in one subtree.
>
> This patch drops map__for_each_symbol_by_name(). Since there is no
> rbtree again, re-matching all symbols costs a lot. This patch avoid it
> by saving all matching results into an array (syms).
>
> The second problem is the lost of tp->realname. In
> __add_probe_trace_events(), if pev->point.function is glob, the event
> name should be set to tev->point.realname. This patch ensures its
> existence by strdup sym->name instead of leaving a NULL pointer there.
>
> After this patch:
>
> # perf probe 'sys_w*'
> Added new events:
> probe:sys_waitid (on sys_w*)
The second part looks strange :-\ Masami?
> probe:sys_wait4 (on sys_w*)
> probe:sys_waitpid (on sys_w*)
> probe:sys_write (on sys_w*)
> probe:sys_writev (on sys_w*)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:sys_writev -aR sleep 1
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> ---
> tools/perf/util/probe-event.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 9052e7b..4f1bc78 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2493,7 +2493,8 @@ close_out:
> return ret;
> }
>
> -static int find_probe_functions(struct map *map, char *name)
> +static int find_probe_functions(struct map *map, char *name,
> + struct symbol **syms)
> {
> int found = 0;
> struct symbol *sym;
> @@ -2503,8 +2504,11 @@ static int find_probe_functions(struct map *map, char *name)
> return 0;
>
> map__for_each_symbol(map, sym, tmp) {
> - if (strglobmatch(sym->name, name))
> + if (strglobmatch(sym->name, name)) {
> found++;
> + if (syms && found < probe_conf.max_probes)
> + syms[found - 1] = sym;
> + }
> }
>
> return found;
> @@ -2527,11 +2531,12 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
> struct map *map = NULL;
> struct ref_reloc_sym *reloc_sym = NULL;
> struct symbol *sym;
> + struct symbol **syms = NULL;
> struct probe_trace_event *tev;
> struct perf_probe_point *pp = &pev->point;
> struct probe_trace_point *tp;
> int num_matched_functions;
> - int ret, i;
> + int ret, i, j;
>
> map = get_target_map(pev->target, pev->uprobes);
> if (!map) {
> @@ -2539,11 +2544,17 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
> goto out;
> }
>
> + syms = malloc(sizeof(struct symbol *) * probe_conf.max_probes);
> + if (!syms) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> /*
> * Load matched symbols: Since the different local symbols may have
> * same name but different addresses, this lists all the symbols.
> */
> - num_matched_functions = find_probe_functions(map, pp->function);
> + num_matched_functions = find_probe_functions(map, pp->function, syms);
> if (num_matched_functions == 0) {
> pr_err("Failed to find symbol %s in %s\n", pp->function,
> pev->target ? : "kernel");
> @@ -2574,7 +2585,9 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>
> ret = 0;
>
> - map__for_each_symbol_by_name(map, pp->function, sym) {
> + for (j = 0; j < num_matched_functions; j++) {
> + sym = syms[j];
> +
> tev = (*tevs) + ret;
> tp = &tev->point;
> if (ret == num_matched_functions) {
> @@ -2598,6 +2611,8 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
> tp->symbol = strdup_or_goto(sym->name, nomem_out);
> tp->offset = pp->offset;
> }
> + tp->realname = strdup_or_goto(sym->name, nomem_out);
> +
> tp->retprobe = pp->retprobe;
> if (pev->target)
> tev->point.module = strdup_or_goto(pev->target,
> @@ -2628,6 +2643,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>
> out:
> put_target_map(map, pev->uprobes);
> + free(syms);
> return ret;
>
> nomem_out:
> --
> 1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists