linux-kernel - Re: [PATCH v3 2/9] perf tools: splice events onto evlist even on error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Oct 2019 10:01:42 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andi Kleen <ak@...ux.intel.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        John Garry <john.garry@...wei.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org, clang-built-linux@...glegroups.com,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v3 2/9] perf tools: splice events onto evlist even on
 error

On Thu, Oct 24, 2019 at 12:01:55PM -0700, Ian Rogers wrote:
> If event parsing fails the event list is leaked, instead splice the list
> onto the out result and let the caller cleanup.
> 
> An example input for parse_events found by libFuzzer that reproduces
> this memory leak is 'm{'.
> 
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
>  tools/perf/util/parse-events.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index edb3ae76777d..f0d50f079d2f 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1968,15 +1968,20 @@ int parse_events(struct evlist *evlist, const char *str,
>  
>  	ret = parse_events__scanner(str, &parse_state, PE_START_EVENTS);
>  	perf_pmu__parse_cleanup();
> +
> +	if (!ret && list_empty(&parse_state.list)) {
> +		WARN_ONCE(true, "WARNING: event parser found nothing\n");
> +		return -1;
> +	}
> +
> +	/*
> +	 * Add list to the evlist even with errors to allow callers to clean up.
> +	 */
> +	perf_evlist__splice_list_tail(evlist, &parse_state.list);

I still dont understand this one.. if there was an error, the list
should be empty, right? also if there's an error and there's something
on the list, what is it? how it gets deleted?

thanks,
jirka

> +
>  	if (!ret) {
>  		struct evsel *last;
>  
> -		if (list_empty(&parse_state.list)) {
> -			WARN_ONCE(true, "WARNING: event parser found nothing\n");
> -			return -1;
> -		}
> -
> -		perf_evlist__splice_list_tail(evlist, &parse_state.list);
>  		evlist->nr_groups += parse_state.nr_groups;
>  		last = evlist__last(evlist);
>  		last->cmdline_group_boundary = true;
> -- 
> 2.23.0.866.gb869b98d4c-goog
>