linux-kernel - Re: [PATCH] Revert "perf tools: Fix PMU term format max value calculation"

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]

Message-ID: <20181009135226.GC10775@kernel.org>
Date:   Tue, 9 Oct 2018 10:52:26 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Andi Kleen <ak@...ux.intel.com>,
        Kan Liang <kan.liang@...el.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andi Kleen <andi@...stfloor.org>,
        Michael Petlan <mpetlan@...hat.com>
Subject: Re: [PATCH] Revert "perf tools: Fix PMU term format max value
 calculation"

Em Tue, Oct 09, 2018 at 11:59:17AM +0200, Jiri Olsa escreveu:
> ping

oops, applied to perf/urgent, added a Fixes: line so that stable@ pick
this up.

- Arnaldo
 
> thanks,
> jirka
> 
> On Wed, Oct 03, 2018 at 09:20:46AM +0200, Jiri Olsa wrote:
> > This reverts commit ac0e2cd555373ae6f8f3a3ad3fbbf5b6d1e7aaaa.
> > 
> > Michael reported an issue with oversized terms values assignment
> > and I noticed there was actually a misunderstanding of the max
> > value check in the past.
> > 
> > The above commit's changelog says:
> > 
> >   If bit 21 is set, there is parsing issues as below.
> > 
> >     $ perf stat -a -e uncore_qpi_0/event=0x200002,umask=0x8/
> >     event syntax error: '..pi_0/event=0x200002,umask=0x8/'
> >                                       \___ value too big for format, maximum is 511
> > 
> > But there's no issue there, because the event value is distributed
> > along the value defined by the format. Even if the format defines
> > separated bit, the value is treated as a continual number, which
> > should follow the format definition.
> > 
> > In above case it's 9-bit value with last bit separated:
> >   $ cat uncore_qpi_0/format/event
> >   config:0-7,21
> > 
> > Hence the value 0x200002 is correctly reported as format violation,
> > because it exceeds 9 bits. It should have been 0x102 instead, which
> > sets the 9th bit - the bit 21 of the format.
> > 
> >   $ perf stat -vv -a -e uncore_qpi_0/event=0x102,umask=0x8/
> >   Using CPUID GenuineIntel-6-2D
> >   ...
> >   ------------------------------------------------------------
> >   perf_event_attr:
> >     type                             10
> >     size                             112
> >     config                           0x200802
> >     sample_type                      IDENTIFIER
> >   ...
> > 
> > Cc: Andi Kleen <ak@...ux.intel.com>
> > Cc: Kan Liang <kan.liang@...el.com>
> > Reported-by: Michael Petlan <mpetlan@...hat.com>
> > Link: http://lkml.kernel.org/n/tip-icxq7a1r66lusm3ahaimekis@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >  tools/perf/util/pmu.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index afd68524ffa9..7799788f662f 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -930,13 +930,14 @@ static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
> >  
> >  static __u64 pmu_format_max_value(const unsigned long *format)
> >  {
> > -	__u64 w = 0;
> > -	int fbit;
> > -
> > -	for_each_set_bit(fbit, format, PERF_PMU_FORMAT_BITS)
> > -		w |= (1ULL << fbit);
> > +	int w;
> >  
> > -	return w;
> > +	w = bitmap_weight(format, PERF_PMU_FORMAT_BITS);
> > +	if (!w)
> > +		return 0;
> > +	if (w < 64)
> > +		return (1ULL << w) - 1;
> > +	return -1;
> >  }
> >  
> >  /*
> > -- 
> > 2.17.1
> >