| 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
| ||
|
Message-ID: <5645A0F6.1080501@huawei.com>
Date: Fri, 13 Nov 2015 16:36:06 +0800
From: "Wangnan (F)" <wangnan0@...wei.com>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
<linux-kernel@...r.kernel.org>
CC: Arnaldo Carvalho de Melo <acme@...hat.com>,
Zefan Li <lizefan@...wei.com>, <pi3orama@....com>
Subject: Re: [PATCH perf/core ] [BUGFIX] perf probe: Fix memory leaking on
faiulre by clearing all probe_trace_events
Hi Masami,
Today I remember the reason why I introduced patch [1]. Although your
patch is correct,
either [1] or [2] is still required, but they are both need to be fixed.
Here is a bug:
A segfault raises if use glob matching and argument together and one of
probe point
failed to get that argument:
# ./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)
Here is how the segfault happen:
In following call stack:
add_probe_trace_event
call_probe_finder
probe_point_search_cb
??
dwarf_getfuncs
find_probe_point_by_func
debuginfo__find_probes
debuginfo__find_trace_events
try_to_find_probe_trace_events
convert_to_probe_trace_events
convert_perf_probe_events
perf_add_probe_events
add_probe_trace_event get called and failed due to failure of
find_variable(). In this case
tev->args is not freed and tev->nargs are positive (1 in this case).
Error from find_variable()
will be passed down along the call stack through return values until get to
probe_point_search_cb(). The problem is that, probe_point_search_cb
doesn't return error
if the function name is a glob:
/* Inlined function: search instances */
param->retval = die_walk_instances(sp_die,
probe_point_inline_cb, (void *)pf);
/* This could be a non-existed inline definition */
if (param->retval == -ENOENT && strisglob(pp->function))
param->retval = 0;
So the error won't be transfer to debuginfo__find_trace_events() from
debuginfo__find_probes(),
and your patch won't take effect.
With patch [1], we set argument number to 0 and frees tev->args. With
this fix we can save that
tev. Here's the final 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_dup3_2 (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
# PAGER=cat ./perf probe -l
probe:SyS_dup2 (on SyS_dup2@...ux-hydrogen/fs/file.c with oldfd)
probe:SyS_dup3 (on SyS_dup3@...ux-hydrogen/fs/file.c with oldfd)
probe:SyS_dup3_1 (on SYSC_dup2:12@...ux-hydrogen/fs/file.c)
probe:SyS_dup3_2 (on SyS_dup3@...ux-hydrogen/fs/file.c with oldfd)
Perf can't find oldfd in the context of SyS_dup3_1. With [1]'s fix it still
probe at it but doesn't create argument fetcher.
[2]'s fix totally clear this tev. It is incorrect because it forgets to
adjust
tf->ntevs. With that appended the final result becomes:
# ./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
# PAGER=cat ./perf probe -l
probe:SyS_dup2 (on SyS_dup2@...ux-hydrogen/fs/file.c with oldfd)
probe:SyS_dup3 (on SyS_dup3@...ux-hydrogen/fs/file.c with oldfd)
probe:SyS_dup3_1 (on SyS_dup3@...ux-hydrogen/fs/file.c with oldfd)
Here only those probe point we can get 'oldfd' is probed.
I suggest [2]'s fix because I think if user provide argument he or she would
get interested with it, and we have already noticed user the argument is
unabled
to be found in one probe point.
I'll provide another patch based on [2]. Please have a look.
Thank you.
[1]
http://lkml.kernel.org/g/1444826502-49291-13-git-send-email-wangnan0@huawei.com
[2]
http://lkml.kernel.org/g/1447237183-106610-1-git-send-email-wangnan0@huawei.com
On 2015/11/11 22:38, Masami Hiramatsu wrote:
> Fix memory leaking on failure path in debuginfo__find_trace_events()
> which frees an array of probe_trace_events but doesn't clear all
> allocated sub-structures and strings.
> So, before doing zfree(tevs), clear all the array elements which
> can have allocated resources.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Reported-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
> tools/perf/util/probe-finder.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index bd8f03d..63993d7 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1246,7 +1246,7 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
> struct trace_event_finder tf = {
> .pf = {.pev = pev, .callback = add_probe_trace_event},
> .max_tevs = probe_conf.max_probes, .mod = dbg->mod};
> - int ret;
> + int ret, i;
>
> /* Allocate result tevs array */
> *tevs = zalloc(sizeof(struct probe_trace_event) * tf.max_tevs);
> @@ -1258,6 +1258,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
>
> ret = debuginfo__find_probes(dbg, &tf.pf);
> if (ret < 0) {
> + for (i = 0; i < tf.ntevs; i++)
> + clear_probe_trace_event(&tf.tevs[i]);
> zfree(tevs);
> return ret;
> }
>
--
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