perf parse-events: fix memory leaks found on parse_events

Memory leaks found by applying LLVM's libfuzzer on the tools/perf parse_events function.

Signed-off-by Ian Rogers <[email protected]>

---
 tools/lib/perf/evlist.c | 2 ++
 1 file changed, 2 insertions(+)

tools/lib/perf/evlist.c +2 -0

index 5b9f2ca50591..6485d1438f75 100644
@@ -125,8 +125,10 @@ static void perf_evlist__purge(struct perf_evlist *evlist)
 void perf_evlist__exit(struct perf_evlist *evlist)
 {
 	perf_cpu_map__put(evlist->cpus);
+	perf_cpu_map__put(evlist->all_cpus);
 	perf_thread_map__put(evlist->threads);
 	evlist->cpus = NULL;
+	evlist->all_cpus = NULL;
 	evlist->threads = NULL;
 	fdarray__exit(&evlist->pollfd);
 }

Signature · 1 line

2.25.1.696.g5e7596f4ac-goog

Memory leaks found by applying LLVM's libfuzzer on the parse_events function.

Signed-off-by Ian Rogers <[email protected]>

---
 tools/perf/util/parse-events.c | 2 ++
 tools/perf/util/parse-events.y | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

tools/perf/util/parse-events.c +2 -0

index 593b6b03785d..1e0bec5c0846 100644
@@ -1482,6 +1482,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 		list_for_each_entry_safe(pos, tmp, &config_terms, list) {
 			list_del_init(&pos->list);
+			if (pos->free_str)
+				free(pos->val.str);
 			free(pos);
 		}
 		return -EINVAL;

tools/perf/util/parse-events.y +2 -1

index 94f8bcd83582..8212cc771667 100644
@@ -44,7 +44,7 @@ static void free_list_evsel(struct list_head* list_evsel)
 
 	list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) {
 		list_del_init(&evsel->core.node);
-		perf_evsel__delete(evsel);
+		evsel__delete(evsel);
 	}
 	free(list_evsel);
 }

@@ -326,6 +326,7 @@ PE_NAME opt_pmu_config
 	}
 	parse_events_terms__delete($2);
 	parse_events_terms__delete(orig_terms);
+	free(pattern);
 	free($1);
 	$$ = list;
 #undef CLEANUP_YYABORT

Signature · 1 line

2.25.1.696.g5e7596f4ac-goog

Quoted 4 lines · Ian Rogers

Memory leaks found by applying LLVM's libfuzzer on the parse_events function.

Signed-off-by Ian Rogers <[email protected]>

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Quoted 42 lines

---
 tools/perf/util/parse-events.c | 2 ++
 tools/perf/util/parse-events.y | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

tools/perf/util/parse-events.c +2 -0

index 593b6b03785d..1e0bec5c0846 100644
@@ -1482,6 +1482,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
              list_for_each_entry_safe(pos, tmp, &config_terms, list) {
                      list_del_init(&pos->list);
+                     if (pos->free_str)
+                             free(pos->val.str);
                      free(pos);
              }
              return -EINVAL;

tools/perf/util/parse-events.y +2 -2

index 94f8bcd83582..8212cc771667 100644
@@ -44,7 +44,7 @@ static void free_list_evsel(struct list_head* list_evsel)
 
      list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) {
              list_del_init(&evsel->core.node);
-             perf_evsel__delete(evsel);
+             evsel__delete(evsel);
      }
      free(list_evsel);
 }

@@ -326,6 +326,7 @@ PE_NAME opt_pmu_config
      }
      parse_events_terms__delete($2);
      parse_events_terms__delete(orig_terms);
+     free(pattern);
      free($1);
      $$ = list;
 #undef CLEANUP_YYABORT
--

2.25.1.696.g5e7596f4ac-goog

Memory leaks found by applying LLVM's libfuzzer on the parse_events function.

Signed-off-by Ian Rogers <[email protected]>

---
 tools/perf/util/parse-events.c | 2 ++
 tools/perf/util/parse-events.y | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

tools/perf/util/parse-events.c +0 -0

index 593b6b03785d..1e0bec5c0846 100644

tools/perf/util/parse-events.y +1 -0

@@ -326,6 +326,7 @@ PE_NAME opt_pmu_config
 	}
 	parse_events_terms__delete($2);
 	parse_events_terms__delete(orig_terms);
+	free(pattern);
 	free($1);
 	$$ = list;
 #undef CLEANUP_YYABORT

Signature · 1 line

2.25.1.696.g5e7596f4ac-goog

Em Wed, Mar 18, 2020 at 07:31:00PM -0700, Ian Rogers escreveu:

Quoted 14 lines

Memory leaks found by applying LLVM's libfuzzer on the parse_events function.

Signed-off-by Ian Rogers <[email protected]>

---
 tools/perf/util/parse-events.c | 2 ++
 tools/perf/util/parse-events.y | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

tools/perf/util/parse-events.c +0 -0

index 593b6b03785d..1e0bec5c0846 100644

tools/perf/util/parse-events.c int parse_events_add_pmu(struct parse_events_state *parse_state, · line 1486

Quoted 5 lines

tools/perf/util/parse-events.c +2 -0

@@ -1482,6 +1482,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
              list_for_each_entry_safe(pos, tmp, &config_terms, list) {
                      list_del_init(&pos->list);
+                     if (pos->free_str)
+                             free(pos->val.str);

I'm applying it but only after changing it to zfree(&pos->free_str), to make sure that any othe rcode that may still hold a pointer to pos will see a NULL in ->free_str and crash sooner rather than later.

tools/perf/util/parse-events.c int parse_events_add_pmu(struct parse_events_state *parse_state, · line 1489

Quoted 3 lines

tools/perf/util/parse-events.c +0 -0

                      free(pos);
              }
              return -EINVAL;

And the following should be in a different patch

tools/perf/util/parse-events.y static void free_list_evsel(struct list_head* list_evsel) · line 50

Quoted 13 lines

tools/perf/util/parse-events.y +1 -1

index 94f8bcd83582..8212cc771667 100644
@@ -44,7 +44,7 @@ static void free_list_evsel(struct list_head* list_evsel)
 
      list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) {
              list_del_init(&evsel->core.node);
-             perf_evsel__delete(evsel);
+             evsel__delete(evsel);
      }
      free(list_evsel);
 }

And this one in another, I'll fix this up, but please try in the future to provide different patches for different fixes, so that if we eventually find out that one of the unrelated fixes is wrong, then we can revert the patch more easily with 'git revert' instead of having to do a patch that reverts just part of the bigger hodge-podge patch.

If you go and have a track record of doing this as piecemeal as possible, I will in turn feel more confident of processing your patches in a faster fashion ;-) :-)

Thanks,

- Arnaldo

Quoted 11 lines

tools/perf/util/parse-events.y +1 -1

@@ -326,6 +326,7 @@ PE_NAME opt_pmu_config
      }
      parse_events_terms__delete($2);
      parse_events_terms__delete(orig_terms);
+     free(pattern);
      free($1);
      $$ = list;
 #undef CLEANUP_YYABORT
--

2.25.1.696.g5e7596f4ac-goog

--

- Arnaldo

Quoted 52 lines · Arnaldo Carvalho de Melo

Em Wed, Mar 18, 2020 at 07:31:00PM -0700, Ian Rogers escreveu:

Quoted 14 lines

Memory leaks found by applying LLVM's libfuzzer on the parse_events function.

Signed-off-by Ian Rogers <[email protected]>

---
 tools/perf/util/parse-events.c | 2 ++
 tools/perf/util/parse-events.y | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

tools/perf/util/parse-events.c +0 -0

index 593b6b03785d..1e0bec5c0846 100644

tools/perf/util/parse-events.c int parse_events_add_pmu(struct parse_events_state *parse_state, · line 1486

Quoted 5 lines

tools/perf/util/parse-events.c +2 -0

@@ -1482,6 +1482,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
              list_for_each_entry_safe(pos, tmp, &config_terms, list) {
                      list_del_init(&pos->list);
+                     if (pos->free_str)
+                             free(pos->val.str);

I'm applying it but only after changing it to zfree(&pos->free_str), to make sure that any othe rcode that may still hold a pointer to pos will see a NULL in ->free_str and crash sooner rather than later.

tools/perf/util/parse-events.c int parse_events_add_pmu(struct parse_events_state *parse_state, · line 1489

Quoted 3 lines

tools/perf/util/parse-events.c +0 -0

                      free(pos);
              }
              return -EINVAL;

And the following should be in a different patch

tools/perf/util/parse-events.y static void free_list_evsel(struct list_head* list_evsel) · line 50

Quoted 13 lines

tools/perf/util/parse-events.y +1 -1

index 94f8bcd83582..8212cc771667 100644
@@ -44,7 +44,7 @@ static void free_list_evsel(struct list_head* list_evsel)
 
      list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) {
              list_del_init(&evsel->core.node);
-             perf_evsel__delete(evsel);
+             evsel__delete(evsel);
      }
      free(list_evsel);
 }

And this one in another, I'll fix this up, but please try in the future to provide different patches for different fixes, so that if we eventually find out that one of the unrelated fixes is wrong, then we can revert the patch more easily with 'git revert' instead of having to do a patch that reverts just part of the bigger hodge-podge patch.

Quoted 3 lines

If you go and have a track record of doing this as piecemeal as possible, I will in turn feel more confident of processing your patches in a faster fashion ;-) :-)

Thanks, at some point I'd like to get libfuzzer with asan working for more than just me so that we don't backslide. It'd also make the reproductions easier to share.