The commit 5a62257a3ddd1 ("perf probe: Replace line_list with
intlist") replaced line_list to intlist but it has a problem that if a
same line was added again, it'd return -EEXIST rather than 1.
Since line_range_walk_cb() only checks the result being negative, it
resulted in failure or segfault sometimes.
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/probe-finder.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index df0238654698..3bf0c8cdccb7 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1441,13 +1441,15 @@ static int line_range_walk_cb(const char *fname, int lineno,
void *data)
{
struct line_finder *lf = data;
+ int err;
if ((strtailcmp(fname, lf->fname) != 0) ||
(lf->lno_s > lineno || lf->lno_e < lineno))
return 0;
- if (line_range_add_line(fname, lineno, lf->lr) < 0)
- return -EINVAL;
+ err = line_range_add_line(fname, lineno, lf->lr);
+ if (err < 0 && err != -EEXIST)
+ return err;
return 0;
}
--
1.7.11.7
(2014/04/01 13:47), Namhyung Kim wrote:
> The commit 5a62257a3ddd1 ("perf probe: Replace line_list with
> intlist") replaced line_list to intlist but it has a problem that if a
> same line was added again, it'd return -EEXIST rather than 1.
Ah, right! that's a different behavior.
> Since line_range_walk_cb() only checks the result being negative, it
> resulted in failure or segfault sometimes.
Could you give me an example input of the segfault? I'd like to trace
it down.
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Thank you very much!
> ---
> tools/perf/util/probe-finder.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index df0238654698..3bf0c8cdccb7 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1441,13 +1441,15 @@ static int line_range_walk_cb(const char *fname, int lineno,
> void *data)
> {
> struct line_finder *lf = data;
> + int err;
>
> if ((strtailcmp(fname, lf->fname) != 0) ||
> (lf->lno_s > lineno || lf->lno_e < lineno))
> return 0;
>
> - if (line_range_add_line(fname, lineno, lf->lr) < 0)
> - return -EINVAL;
> + err = line_range_add_line(fname, lineno, lf->lr);
> + if (err < 0 && err != -EEXIST)
> + return err;
>
> return 0;
> }
>
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
Hi Masami,
(Also adding Jiri in CC and changing email of acme)
On Tue, Apr 1, 2014 at 5:01 AM, Masami Hiramatsu
<[email protected]> wrote:
> (2014/04/01 13:47), Namhyung Kim wrote:
>> The commit 5a62257a3ddd1 ("perf probe: Replace line_list with
>> intlist") replaced line_list to intlist but it has a problem that if a
>> same line was added again, it'd return -EEXIST rather than 1.
>
> Ah, right! that's a different behavior.
>
>> Since line_range_walk_cb() only checks the result being negative, it
>> resulted in failure or segfault sometimes.
>
> Could you give me an example input of the segfault? I'd like to trace
> it down.
Just used current acme/perf/core.
$ git log -1
commit 28b5724a61cc9d84f2cbef6675e8a85ae4b1bc57
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Thu Mar 27 17:36:14 2014 -0300
MAINTAINERS: Change e-mail to kernel.org one
Leaving ghostprotocols.net for old networking stuff.
...
$ perf --version
perf version 3.14.rc6.g28b572
$ perf probe -x ./perf -v -L map__load
Open Debuginfo file: /home/namhyung/project/linux/tools/perf/perf
fname: util/map.c, lineno:153
New line range: 153 to 2147483647
path: (null)
Segmentation fault (core dumped)
It seems like find_line_range_by_line() called for multiple times,
line_range_walk_cb returns 0 at first but as line_range_add_line()
returns -EEXIST, it freed lr->path and __show_line_range() ended up
accessing to NULL path.
(gdb) bt
#0 get_real_path (new_path=0x839ff8 <params+9272>,
comp_dir=0x1f994f0 "/home/namhyung/project/linux/tools/perf",
raw_path=0x0) at util/probe-event.c:518
#1 __show_line_range (module=<optimized out>, lr=0x839fd8 <params+9240>)
at util/probe-event.c:641
#2 show_line_range (lr=lr@entry=0x839fd8 <params+9240>,
module=<optimized out>) at util/probe-event.c:699
#3 0x000000000044667b in __cmd_probe (argc=<optimized out>,
argv=argv@entry=0x7fff7a770ba0, prefix=<optimized out>)
at builtin-probe.c:465
#4 0x00000000004469f4 in cmd_probe (argc=<optimized out>,
argv=0x7fff7a770ba0, prefix=<optimized out>) at builtin-probe.c:520
#5 0x00000000004241b5 in run_builtin (p=p@entry=0x7ebde0 <commands+384>,
argc=argc@entry=6, argv=argv@entry=0x7fff7a770ba0) at perf.c:319
#6 0x0000000000423a29 in handle_internal_command (argv=0x7fff7a770ba0,
argc=6) at perf.c:376
#7 run_argv (argv=0x7fff7a7709a0, argcp=0x7fff7a7709ac) at perf.c:420
#8 main (argc=6, argv=0x7fff7a770ba0) at perf.c:529
Thanks,
Namhyung
(2014/04/01 16:21), Namhyung Kim wrote:
> Hi Masami,
>
> (Also adding Jiri in CC and changing email of acme)
>
> On Tue, Apr 1, 2014 at 5:01 AM, Masami Hiramatsu
> <[email protected]> wrote:
>> (2014/04/01 13:47), Namhyung Kim wrote:
>>> The commit 5a62257a3ddd1 ("perf probe: Replace line_list with
>>> intlist") replaced line_list to intlist but it has a problem that if a
>>> same line was added again, it'd return -EEXIST rather than 1.
>>
>> Ah, right! that's a different behavior.
>>
>>> Since line_range_walk_cb() only checks the result being negative, it
>>> resulted in failure or segfault sometimes.
>>
>> Could you give me an example input of the segfault? I'd like to trace
>> it down.
>
> Just used current acme/perf/core.
>
> $ git log -1
> commit 28b5724a61cc9d84f2cbef6675e8a85ae4b1bc57
> Author: Arnaldo Carvalho de Melo <[email protected]>
> Date: Thu Mar 27 17:36:14 2014 -0300
>
> MAINTAINERS: Change e-mail to kernel.org one
>
> Leaving ghostprotocols.net for old networking stuff.
>
> ...
>
> $ perf --version
> perf version 3.14.rc6.g28b572
>
> $ perf probe -x ./perf -v -L map__load
> Open Debuginfo file: /home/namhyung/project/linux/tools/perf/perf
> fname: util/map.c, lineno:153
> New line range: 153 to 2147483647
> path: (null)
> Segmentation fault (core dumped)
>
>
> It seems like find_line_range_by_line() called for multiple times,
> line_range_walk_cb returns 0 at first but as line_range_add_line()
> returns -EEXIST, it freed lr->path and __show_line_range() ended up
> accessing to NULL path.
OK, thanks for this nice report!
Hmm, line_range_inline_cb() must check the result of
find_line_range_by_line() but ignored. That should be fixed too...
Thank you,
>
>
> (gdb) bt
> #0 get_real_path (new_path=0x839ff8 <params+9272>,
> comp_dir=0x1f994f0 "/home/namhyung/project/linux/tools/perf",
> raw_path=0x0) at util/probe-event.c:518
> #1 __show_line_range (module=<optimized out>, lr=0x839fd8 <params+9240>)
> at util/probe-event.c:641
> #2 show_line_range (lr=lr@entry=0x839fd8 <params+9240>,
> module=<optimized out>) at util/probe-event.c:699
> #3 0x000000000044667b in __cmd_probe (argc=<optimized out>,
> argv=argv@entry=0x7fff7a770ba0, prefix=<optimized out>)
> at builtin-probe.c:465
> #4 0x00000000004469f4 in cmd_probe (argc=<optimized out>,
> argv=0x7fff7a770ba0, prefix=<optimized out>) at builtin-probe.c:520
> #5 0x00000000004241b5 in run_builtin (p=p@entry=0x7ebde0 <commands+384>,
> argc=argc@entry=6, argv=argv@entry=0x7fff7a770ba0) at perf.c:319
> #6 0x0000000000423a29 in handle_internal_command (argv=0x7fff7a770ba0,
> argc=6) at perf.c:376
> #7 run_argv (argv=0x7fff7a7709a0, argcp=0x7fff7a7709ac) at perf.c:420
> #8 main (argc=6, argv=0x7fff7a770ba0) at perf.c:529
>
>
> Thanks,
> Namhyung
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
As Namhyung reported(https://lkml.org/lkml/2014/4/1/89),
current perf-probe -L option doesn't handle errors in line-range
searching correctly. It causes a SEGV if an error occured in the
line-range searching.
----
$ perf probe -x ./perf -v -L map__load
Open Debuginfo file: /home/namhyung/project/linux/tools/perf/perf
fname: util/map.c, lineno:153
New line range: 153 to 2147483647
path: (null)
Segmentation fault (core dumped)
----
This is because line_range_inline_cb() ignores errors
from find_line_range_by_line() which means that lr->path is
already freed on the error path in find_line_range_by_line().
As a result, get_real_path() accesses the lr->path and it
causes a NULL pointer exception.
This fixes line_range_inline_cb() to handle the error correctly,
and report it to the caller.
Anyway, this just fixes a possible SEGV bug, Namhyung's patch
is also required.
Signed-off-by: Masami Hiramatsu <[email protected]>
Reported-by: Namhyung Kim <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Jiri Olsa <[email protected]>
---
tools/perf/util/probe-finder.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index df02386..edcd2ff 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1473,14 +1473,15 @@ static int find_line_range_by_line(Dwarf_Die *sp_die, struct line_finder *lf)
static int line_range_inline_cb(Dwarf_Die *in_die, void *data)
{
- find_line_range_by_line(in_die, data);
+ int ret = find_line_range_by_line(in_die, data);
/*
* We have to check all instances of inlined function, because
* some execution paths can be optimized out depends on the
- * function argument of instances
+ * function argument of instances. However, if an error occurs,
+ * it should be handled by the caller.
*/
- return 0;
+ return ret < 0 ? ret : 0;
}
/* Search function definition from function name */
On Wed, Apr 02, 2014 at 02:48:31PM +0900, Masami Hiramatsu wrote:
> As Namhyung reported(https://lkml.org/lkml/2014/4/1/89),
> current perf-probe -L option doesn't handle errors in line-range
> searching correctly. It causes a SEGV if an error occured in the
> line-range searching.
>
> ----
> $ perf probe -x ./perf -v -L map__load
> Open Debuginfo file: /home/namhyung/project/linux/tools/perf/perf
> fname: util/map.c, lineno:153
> New line range: 153 to 2147483647
> path: (null)
> Segmentation fault (core dumped)
> ----
>
> This is because line_range_inline_cb() ignores errors
> from find_line_range_by_line() which means that lr->path is
> already freed on the error path in find_line_range_by_line().
> As a result, get_real_path() accesses the lr->path and it
> causes a NULL pointer exception.
>
> This fixes line_range_inline_cb() to handle the error correctly,
> and report it to the caller.
>
> Anyway, this just fixes a possible SEGV bug, Namhyung's patch
> is also required.
will take both.. Namhyung, ack to Masami's change?
thanks,
jirka
On Sun, 6 Apr 2014 14:52:48 +0200, Jiri Olsa wrote:
> On Wed, Apr 02, 2014 at 02:48:31PM +0900, Masami Hiramatsu wrote:
>> As Namhyung reported(https://lkml.org/lkml/2014/4/1/89),
>> current perf-probe -L option doesn't handle errors in line-range
>> searching correctly. It causes a SEGV if an error occured in the
>> line-range searching.
>>
>> ----
>> $ perf probe -x ./perf -v -L map__load
>> Open Debuginfo file: /home/namhyung/project/linux/tools/perf/perf
>> fname: util/map.c, lineno:153
>> New line range: 153 to 2147483647
>> path: (null)
>> Segmentation fault (core dumped)
>> ----
>>
>> This is because line_range_inline_cb() ignores errors
>> from find_line_range_by_line() which means that lr->path is
>> already freed on the error path in find_line_range_by_line().
>> As a result, get_real_path() accesses the lr->path and it
>> causes a NULL pointer exception.
>>
>> This fixes line_range_inline_cb() to handle the error correctly,
>> and report it to the caller.
>>
>> Anyway, this just fixes a possible SEGV bug, Namhyung's patch
>> is also required.
>
> will take both.. Namhyung, ack to Masami's change?
Yes,
Acked-by: Namhyung Kim <[email protected]>
Thanks,
Namhyung
Commit-ID: 182c228ebcf1ac67a44e62236d8f7a8a9a3c5699
Gitweb: http://git.kernel.org/tip/182c228ebcf1ac67a44e62236d8f7a8a9a3c5699
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 2 Apr 2014 14:48:31 +0900
Committer: Jiri Olsa <[email protected]>
CommitDate: Mon, 14 Apr 2014 12:55:39 +0200
perf probe: Fix to handle errors in line_range searching
As Namhyung reported(https://lkml.org/lkml/2014/4/1/89),
current perf-probe -L option doesn't handle errors in line-range
searching correctly. It causes a SEGV if an error occured in the
line-range searching.
----
$ perf probe -x ./perf -v -L map__load
Open Debuginfo file: /home/namhyung/project/linux/tools/perf/perf
fname: util/map.c, lineno:153
New line range: 153 to 2147483647
path: (null)
Segmentation fault (core dumped)
----
This is because line_range_inline_cb() ignores errors
from find_line_range_by_line() which means that lr->path is
already freed on the error path in find_line_range_by_line().
As a result, get_real_path() accesses the lr->path and it
causes a NULL pointer exception.
This fixes line_range_inline_cb() to handle the error correctly,
and report it to the caller.
Anyway, this just fixes a possible SEGV bug, Namhyung's patch
is also required.
Reported-by: Namhyung Kim <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/probe-finder.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 3bf0c8c..fae274e 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1475,14 +1475,15 @@ static int find_line_range_by_line(Dwarf_Die *sp_die, struct line_finder *lf)
static int line_range_inline_cb(Dwarf_Die *in_die, void *data)
{
- find_line_range_by_line(in_die, data);
+ int ret = find_line_range_by_line(in_die, data);
/*
* We have to check all instances of inlined function, because
* some execution paths can be optimized out depends on the
- * function argument of instances
+ * function argument of instances. However, if an error occurs,
+ * it should be handled by the caller.
*/
- return 0;
+ return ret < 0 ? ret : 0;
}
/* Search function definition from function name */
Commit-ID: 202c7c123c96a1c193149b7fa2718d7fb143efb2
Gitweb: http://git.kernel.org/tip/202c7c123c96a1c193149b7fa2718d7fb143efb2
Author: Namhyung Kim <[email protected]>
AuthorDate: Tue, 1 Apr 2014 13:47:57 +0900
Committer: Jiri Olsa <[email protected]>
CommitDate: Mon, 14 Apr 2014 12:55:39 +0200
perf probe: Fix --line option behavior
The commit 5a62257a3ddd1 ("perf probe: Replace line_list with
intlist") replaced line_list to intlist but it has a problem that if a
same line was added again, it'd return -EEXIST rather than 1.
Since line_range_walk_cb() only checks the result being negative, it
resulted in failure or segfault sometimes.
Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/probe-finder.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index df02386..3bf0c8c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1441,13 +1441,15 @@ static int line_range_walk_cb(const char *fname, int lineno,
void *data)
{
struct line_finder *lf = data;
+ int err;
if ((strtailcmp(fname, lf->fname) != 0) ||
(lf->lno_s > lineno || lf->lno_e < lineno))
return 0;
- if (line_range_add_line(fname, lineno, lf->lr) < 0)
- return -EINVAL;
+ err = line_range_add_line(fname, lineno, lf->lr);
+ if (err < 0 && err != -EEXIST)
+ return err;
return 0;
}