linux-kernel - Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv

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] [day] [month] [year] [list]

Message-Id: <6691adb2-e3d7-3cae-1d78-e5fd30317283@linux.ibm.com>
Date:   Fri, 22 Jun 2018 09:23:50 +0530
From:   Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     jolsa@...hat.com, alexander.shishkin@...ux.intel.com,
        namhyung@...nel.org, dsahern@...il.com, davidcc@...gle.com,
        ak@...ux.intel.com, yao.jin@...ux.intel.com,
        linux-kernel@...r.kernel.org,
        Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Subject: Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv

Hi Arnaldo,

On 06/20/2018 07:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
>> perf script in pipped mode is crashing because evsel->priv is not
>> set properly. Fix it.
>>
>> Before:
>>   # ./perf record -o - -- ls | ./perf script
>>     Segmentation fault (core dumped)
>>
>> After:
>>   # ./perf record -o - -- ls | ./perf script
>>   ls 2282 1031.731974:  250000 cpu-clock:uhH:  7effe4b3d29e
>>   ls 2282 1031.732222:  250000 cpu-clock:uhH:  7effe4b3a650
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
>> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
> 
> Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
> object, will check which one does to continue review.

So, it's not about setting evsel->priv to 'struct perf_evsel_script', but
it's about setting proper output stream, either file or stdout. When
'struct perf_evsel_script' was not introduced, we were setting evsel->priv
to output stream. So I think, this commit missed to set evsel->priv properly
in pipped case. Ex, below patch applied _directly_ on top of a14390fde64e
fixes the issue.

------
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index fb5e49b3bc44..66cc4b29bf4d 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1645,6 +1645,9 @@ static int process_attr(struct perf_tool *tool, union perf_event *event,
 	evlist = *pevlist;
 	evsel = perf_evlist__last(*pevlist);
 
+	if (!evsel->priv)
+		evsel->priv = stdout;
+
 	if (evsel->attr.type >= PERF_TYPE_MAX &&
 	    evsel->attr.type != PERF_TYPE_SYNTH)
 		return 0;
------

To me this commit seems to be the bad. Please let me know if that is not
the case. I'll change the last patch as you suggested an post v2 soon.

Thanks,
Ravi