linux-kernel - [PATCH] perf tools: Fix locale handling in pmu parsing

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:	Thu, 3 Mar 2016 10:53:48 +0100
From:	Jiri Olsa <jolsa@...hat.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	linux-kernel@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
	Andi Kleen <ak@...ux.intel.com>,
	Brendan Gregg <brendan.d.gregg@...il.com>,
	David Ahern <dsahern@...il.com>, He Kuang <hekuang@...wei.com>,
	Jeff Bastian <jbastian@...hat.com>,
	Jeremie Galarneau <jeremie.galarneau@...icios.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Josh Boyer <jwboyer@...oraproject.org>,
	Lai Jiangshan <jiangshanlai@...il.com>,
	Li Zefan <lizefan@...wei.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>, pi3orama@....com,
	Stephane Eranian <eranian@...gle.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Taeung Song <treeze.taeung@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Wang Nan <wangnan0@...wei.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: [PATCH] perf tools: Fix locale handling in pmu parsing

On Thu, Mar 03, 2016 at 10:15:22AM +0100, Jiri Olsa wrote:

SNIP

> > 
> >       11989.280397      task-clock (msec)         #   11.981 CPUs utilized          
> >               1299      context-switches          #    0.108 K/sec                  
> >                  6      cpu-migrations            #    0.001 K/sec                  
> >                 70      page-faults               #    0.006 K/sec                  
> >          127008602      cycles                    #    0.011 GHz                    
> >          279538533      stalled-cycles-frontend   #  220.09% frontend cycles idle   
> >          119213269      instructions              #    0.94  insn per cycle         
> >                                                   #    2.34  stalled cycles per insn
> >           24166678      branches                  #    2.016 M/sec                  
> >             505681      branch-misses             #    2.09% of all branches        
> > 
> >        1.000684278 seconds time elapsed
> > 
> > 
> > ... see how the numbers became human-unreadable, losing the big-number separator?
> > 
> > I suspect it's due to the following commit:
> > 
> >   fa184776ac27 perf stat: Check existence of frontend/backed stalled cycles
> 
> yea, it used the pmu parsing which screwes locales,
> following patch fixed that for me..
> 

resending with full changelog

jirka


---
Ingo reported regression on display format of big numbers,
which is missing separators (in default perf stat output).

 triton:~/tip> perf stat -a sleep 1
         ...
         127008602      cycles                    #    0.011 GHz
         279538533      stalled-cycles-frontend   #  220.09% frontend cycles idle
         119213269      instructions              #    0.94  insn per cycle

This is caused by recent change:
  perf stat: Check existence of frontend/backed stalled cycles

that added call to pmu_have_event, that subsequently calls
perf_pmu__parse_scale, which has a bug in locale handling.

The lc string returned from setlocale, that we use to store
old locale value, may be allocated in static storage. Getting
a dynamic copy to make it survive another setlocale call.

 [jolsa@...va perf]$ perf stat ls
         ...
         2,360,602      cycles                    #    3.080 GHz
         2,703,090      instructions              #    1.15  insn per cycle
           546,031      branches                  #  712.511 M/sec

Reported-by: Ingo Molnar <mingo@...hat.com>
Cc: David Ahern <dsahern@...il.com>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Signed-off-by: Jiri Olsa <jolsa@...nel.org>
---
 tools/perf/util/pmu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ce61f79dbaae..d8cd038baed2 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -124,6 +124,17 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *
 	lc = setlocale(LC_NUMERIC, NULL);
 
 	/*
+	 * The lc string may be allocated in static storage,
+	 * so get a dynamic copy to make it survive setlocale
+	 * call below.
+	 */
+	lc = strdup(lc);
+	if (!lc) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	/*
 	 * force to C locale to ensure kernel
 	 * scale string is converted correctly.
 	 * kernel uses default C locale.
@@ -135,6 +146,8 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *
 	/* restore locale */
 	setlocale(LC_NUMERIC, lc);
 
+	free((char *) lc);
+
 	ret = 0;
 error:
 	close(fd);
-- 
2.4.3