* [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()
@ 2023-06-30 15:38 James Clark
2023-06-30 15:49 ` Ian Rogers
0 siblings, 1 reply; 3+ messages in thread
From: James Clark @ 2023-06-30 15:38 UTC (permalink / raw)
To: linux-perf-users, irogers
Cc: James Clark, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel
found_idx and s aren't initialized, so if no symbol is found then the
assert at the end will index off the end of the array causing a
segfault. The function also doesn't return NULL when the symbol isn't
found even if the assert passes. Fix it by initializing the values and
only setting them when something is found.
Fixes the following test failure:
$ perf test 1
1: vmlinux symtab matches kallsyms : FAILED!
Fixes: 259dce914e93 ("perf symbol: Remove symbol_name_rb_node")
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/util/symbol.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index bc79291b9f3b..f849f9ef68e6 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -495,7 +495,10 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
size_t *found_idx)
{
size_t i, lower = 0, upper = symbols_len;
- struct symbol *s;
+ struct symbol *s = NULL;
+
+ if (found_idx)
+ *found_idx = SIZE_MAX;
if (!symbols_len)
return NULL;
@@ -504,8 +507,7 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
int cmp;
i = (lower + upper) / 2;
- s = symbols[i];
- cmp = symbol__match_symbol_name(s->name, name, includes);
+ cmp = symbol__match_symbol_name(symbols[i]->name, name, includes);
if (cmp > 0)
upper = i;
@@ -514,10 +516,11 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
else {
if (found_idx)
*found_idx = i;
+ s = symbols[i];
break;
}
}
- if (includes != SYMBOL_TAG_INCLUDE__DEFAULT_ONLY) {
+ if (s && includes != SYMBOL_TAG_INCLUDE__DEFAULT_ONLY) {
/* return first symbol that has same name (if any) */
for (; i > 0; i--) {
struct symbol *tmp = symbols[i - 1];
@@ -525,13 +528,12 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
if (!arch__compare_symbol_names(tmp->name, s->name)) {
if (found_idx)
*found_idx = i - 1;
+ s = tmp;
} else
break;
-
- s = tmp;
}
}
- assert(!found_idx || s == symbols[*found_idx]);
+ assert(!found_idx || !s || s == symbols[*found_idx]);
return s;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()
2023-06-30 15:38 [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name() James Clark
@ 2023-06-30 15:49 ` Ian Rogers
2023-07-02 1:02 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2023-06-30 15:49 UTC (permalink / raw)
To: James Clark
Cc: linux-perf-users, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel
On Fri, Jun 30, 2023 at 8:39 AM James Clark <james.clark@arm.com> wrote:
>
> found_idx and s aren't initialized, so if no symbol is found then the
> assert at the end will index off the end of the array causing a
> segfault. The function also doesn't return NULL when the symbol isn't
> found even if the assert passes. Fix it by initializing the values and
> only setting them when something is found.
>
> Fixes the following test failure:
>
> $ perf test 1
> 1: vmlinux symtab matches kallsyms : FAILED!
>
> Fixes: 259dce914e93 ("perf symbol: Remove symbol_name_rb_node")
> Signed-off-by: James Clark <james.clark@arm.com>
Thanks, and thanks for the Fixes.
Acked-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/symbol.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index bc79291b9f3b..f849f9ef68e6 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -495,7 +495,10 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
> size_t *found_idx)
> {
> size_t i, lower = 0, upper = symbols_len;
> - struct symbol *s;
> + struct symbol *s = NULL;
> +
> + if (found_idx)
> + *found_idx = SIZE_MAX;
>
> if (!symbols_len)
> return NULL;
> @@ -504,8 +507,7 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
> int cmp;
>
> i = (lower + upper) / 2;
> - s = symbols[i];
> - cmp = symbol__match_symbol_name(s->name, name, includes);
> + cmp = symbol__match_symbol_name(symbols[i]->name, name, includes);
>
> if (cmp > 0)
> upper = i;
> @@ -514,10 +516,11 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
> else {
> if (found_idx)
> *found_idx = i;
> + s = symbols[i];
> break;
> }
> }
> - if (includes != SYMBOL_TAG_INCLUDE__DEFAULT_ONLY) {
> + if (s && includes != SYMBOL_TAG_INCLUDE__DEFAULT_ONLY) {
> /* return first symbol that has same name (if any) */
> for (; i > 0; i--) {
> struct symbol *tmp = symbols[i - 1];
> @@ -525,13 +528,12 @@ static struct symbol *symbols__find_by_name(struct symbol *symbols[],
> if (!arch__compare_symbol_names(tmp->name, s->name)) {
> if (found_idx)
> *found_idx = i - 1;
> + s = tmp;
> } else
> break;
> -
> - s = tmp;
> }
> }
> - assert(!found_idx || s == symbols[*found_idx]);
> + assert(!found_idx || !s || s == symbols[*found_idx]);
> return s;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()
2023-06-30 15:49 ` Ian Rogers
@ 2023-07-02 1:02 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2023-07-02 1:02 UTC (permalink / raw)
To: Ian Rogers
Cc: James Clark, linux-perf-users, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Adrian Hunter, linux-kernel
On Fri, Jun 30, 2023 at 8:49 AM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, Jun 30, 2023 at 8:39 AM James Clark <james.clark@arm.com> wrote:
> >
> > found_idx and s aren't initialized, so if no symbol is found then the
> > assert at the end will index off the end of the array causing a
> > segfault. The function also doesn't return NULL when the symbol isn't
> > found even if the assert passes. Fix it by initializing the values and
> > only setting them when something is found.
> >
> > Fixes the following test failure:
> >
> > $ perf test 1
> > 1: vmlinux symtab matches kallsyms : FAILED!
> >
> > Fixes: 259dce914e93 ("perf symbol: Remove symbol_name_rb_node")
> > Signed-off-by: James Clark <james.clark@arm.com>
>
> Thanks, and thanks for the Fixes.
>
> Acked-by: Ian Rogers <irogers@google.com>
Applied to perf-tools-next, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-02 1:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-30 15:38 [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name() James Clark
2023-06-30 15:49 ` Ian Rogers
2023-07-02 1:02 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).