1. Patchew
  2. linux
  3. View series

[PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()

James Clark posted 1 patch 2 years, 7 months ago 
tools/perf/util/symbol.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
[PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()
Posted by James Clark 2 years, 7 months ago 
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
Re: [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()
Posted by Ian Rogers 2 years, 7 months ago 
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
>
Re: [PATCH] perf symbol: Fix uninitialized return value in symbols__find_by_name()
Posted by Namhyung Kim 2 years, 7 months ago 
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!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.