Re: [PATCH v2 1/2] perf parse-events: fix memory leaks found on parse_events

[Arnaldo Carvalho de Melo]

Em Wed, Mar 18, 2020 at 07:31:00PM -0700, Ian Rogers escreveu:
> Memory leaks found by applying LLVM's libfuzzer on the parse_events
> function.
> 
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/perf/util/parse-events.c | 2 ++
>  tools/perf/util/parse-events.y | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 593b6b03785d..1e0bec5c0846 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -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.

>  			free(pos);
>  		}
>  		return -EINVAL;

And the following should be in a different patch

> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 94f8bcd83582..8212cc771667 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -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

> @@ -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