* [PATCH 1/2] perf annotate: Fix segfault on sample histogram [not found] <CAM9d7ci6Y98hkiN1+=Q=YgLQLL-GN+pTxQ1JjqA+Q1EMEi52+g@mail.gmail.com> @ 2024-05-10 21:04 ` Namhyung Kim 2024-05-10 21:04 ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim 2024-05-10 21:27 ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Ian Rogers 0 siblings, 2 replies; 5+ messages in thread From: Namhyung Kim @ 2024-05-10 21:04 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users A symbol can have no samples, then accessing annotated_source->samples hashmap will get a segfault. Fixes: a3f7768bcf48 ("perf annotate: Fix memory leak in annotated_source") Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/annotate.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 541988cf6e19..1451caf25e77 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -113,10 +113,11 @@ static __maybe_unused void annotated_source__delete(struct annotated_source *src if (src == NULL) return; - hashmap__for_each_entry(src->samples, cur, bkt) - zfree(&cur->pvalue); - - hashmap__free(src->samples); + if (src->samples) { + hashmap__for_each_entry(src->samples, cur, bkt) + zfree(&cur->pvalue); + hashmap__free(src->samples); + } zfree(&src->histograms); free(src); } -- 2.45.0.118.g7fe29c98d7-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] perf annotate-data: Ensure the number of type histograms 2024-05-10 21:04 ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim @ 2024-05-10 21:04 ` Namhyung Kim 2024-05-10 21:27 ` Ian Rogers 2024-05-10 21:27 ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Ian Rogers 1 sibling, 1 reply; 5+ messages in thread From: Namhyung Kim @ 2024-05-10 21:04 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users Arnaldo reported that there is a case where nr_histograms and histograms don't agree each other. It ended up in a segfault trying to access NULL histograms array. Let's make sure to update the nr_histograms when the histograms array is changed. Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/annotate-data.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 57e7d4b3550b..965da6c0b542 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en sz += sizeof(struct type_hist_entry) * adt->self.size; /* Allocate a table of pointers for each event */ - adt->nr_histograms = nr_entries; adt->histograms = calloc(nr_entries, sizeof(*adt->histograms)); if (adt->histograms == NULL) return -ENOMEM; @@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en if (adt->histograms[i] == NULL) goto err; } + + adt->nr_histograms = nr_entries; return 0; err: @@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt) { for (int i = 0; i < adt->nr_histograms; i++) zfree(&(adt->histograms[i])); + zfree(&adt->histograms); + adt->nr_histograms = 0; } void annotated_data_type__tree_delete(struct rb_root *root) -- 2.45.0.118.g7fe29c98d7-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] perf annotate-data: Ensure the number of type histograms 2024-05-10 21:04 ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim @ 2024-05-10 21:27 ` Ian Rogers 2024-05-11 15:43 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: Ian Rogers @ 2024-05-10 21:27 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote: > > Arnaldo reported that there is a case where nr_histograms and histograms > don't agree each other. It ended up in a segfault trying to access NULL > histograms array. Let's make sure to update the nr_histograms when the > histograms array is changed. > > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Reviewed-by: Ian Rogers <irogers@google.com> Thanks, Ian > --- > tools/perf/util/annotate-data.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c > index 57e7d4b3550b..965da6c0b542 100644 > --- a/tools/perf/util/annotate-data.c > +++ b/tools/perf/util/annotate-data.c > @@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en > sz += sizeof(struct type_hist_entry) * adt->self.size; > > /* Allocate a table of pointers for each event */ > - adt->nr_histograms = nr_entries; > adt->histograms = calloc(nr_entries, sizeof(*adt->histograms)); > if (adt->histograms == NULL) > return -ENOMEM; > @@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en > if (adt->histograms[i] == NULL) > goto err; > } > + > + adt->nr_histograms = nr_entries; > return 0; > > err: > @@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt) > { > for (int i = 0; i < adt->nr_histograms; i++) > zfree(&(adt->histograms[i])); > + > zfree(&adt->histograms); > + adt->nr_histograms = 0; > } > > void annotated_data_type__tree_delete(struct rb_root *root) > -- > 2.45.0.118.g7fe29c98d7-goog > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] perf annotate-data: Ensure the number of type histograms 2024-05-10 21:27 ` Ian Rogers @ 2024-05-11 15:43 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2024-05-11 15:43 UTC (permalink / raw) To: Ian Rogers Cc: Namhyung Kim, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On Fri, May 10, 2024 at 02:27:36PM -0700, Ian Rogers wrote: > On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > Arnaldo reported that there is a case where nr_histograms and histograms > > don't agree each other. It ended up in a segfault trying to access NULL > > histograms array. Let's make sure to update the nr_histograms when the > > histograms array is changed. > > > > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > Reviewed-by: Ian Rogers <irogers@google.com> Thanks, applied to perf-tools-next, - Arnaldo > Thanks, > Ian > > > --- > > tools/perf/util/annotate-data.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c > > index 57e7d4b3550b..965da6c0b542 100644 > > --- a/tools/perf/util/annotate-data.c > > +++ b/tools/perf/util/annotate-data.c > > @@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en > > sz += sizeof(struct type_hist_entry) * adt->self.size; > > > > /* Allocate a table of pointers for each event */ > > - adt->nr_histograms = nr_entries; > > adt->histograms = calloc(nr_entries, sizeof(*adt->histograms)); > > if (adt->histograms == NULL) > > return -ENOMEM; > > @@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en > > if (adt->histograms[i] == NULL) > > goto err; > > } > > + > > + adt->nr_histograms = nr_entries; > > return 0; > > > > err: > > @@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt) > > { > > for (int i = 0; i < adt->nr_histograms; i++) > > zfree(&(adt->histograms[i])); > > + > > zfree(&adt->histograms); > > + adt->nr_histograms = 0; > > } > > > > void annotated_data_type__tree_delete(struct rb_root *root) > > -- > > 2.45.0.118.g7fe29c98d7-goog > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] perf annotate: Fix segfault on sample histogram 2024-05-10 21:04 ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim 2024-05-10 21:04 ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim @ 2024-05-10 21:27 ` Ian Rogers 1 sibling, 0 replies; 5+ messages in thread From: Ian Rogers @ 2024-05-10 21:27 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote: > > A symbol can have no samples, then accessing annotated_source->samples > hashmap will get a segfault. > > Fixes: a3f7768bcf48 ("perf annotate: Fix memory leak in annotated_source") > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Reviewed-by: Ian Rogers <irogers@google.com> Thanks, Ian > --- > tools/perf/util/annotate.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 541988cf6e19..1451caf25e77 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -113,10 +113,11 @@ static __maybe_unused void annotated_source__delete(struct annotated_source *src > if (src == NULL) > return; > > - hashmap__for_each_entry(src->samples, cur, bkt) > - zfree(&cur->pvalue); > - > - hashmap__free(src->samples); > + if (src->samples) { > + hashmap__for_each_entry(src->samples, cur, bkt) > + zfree(&cur->pvalue); > + hashmap__free(src->samples); > + } > zfree(&src->histograms); > free(src); > } > -- > 2.45.0.118.g7fe29c98d7-goog > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-11 15:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAM9d7ci6Y98hkiN1+=Q=YgLQLL-GN+pTxQ1JjqA+Q1EMEi52+g@mail.gmail.com>
2024-05-10 21:04 ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim
2024-05-10 21:04 ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim
2024-05-10 21:27 ` Ian Rogers
2024-05-11 15:43 ` Arnaldo Carvalho de Melo
2024-05-10 21:27 ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Ian Rogers
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).