linux-kernel - Re: [PATCH v4] perf pmu: Validate raw event with sysfs exported format bits

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: <YEivmv6WCWW9ae59@krava>
Date:   Wed, 10 Mar 2021 12:38:02 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Jin Yao <yao.jin@...ux.intel.com>
Cc:     acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
        mingo@...hat.com, alexander.shishkin@...ux.intel.com,
        Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
        kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v4] perf pmu: Validate raw event with sysfs exported
 format bits

On Wed, Mar 10, 2021 at 01:11:38PM +0800, Jin Yao wrote:
> A raw PMU event (eventsel+umask) in the form of rNNN is supported
> by perf but lacks of checking for the validity of raw encoding.
> 
> For example, bit 16 and bit 17 are not valid on KBL but perf doesn't
> report warning when encoding with these bits.
> 
> Before:
> 
>   # ./perf stat -e cpu/r031234/ -a -- sleep 1
> 
>    Performance counter stats for 'system wide':
> 
>                    0      cpu/r031234/
> 
>          1.003798924 seconds time elapsed
> 
> It may silently measure the wrong event!
> 
> The kernel supported bits have been exported through
> /sys/devices/<pmu>/format/. Perf collects the information to
> 'struct perf_pmu_format' and links it to 'pmu->format' list.
> 
> The 'struct perf_pmu_format' has a bitmap which records the
> valid bits for this format. For example,
> 
>   root@...-ppc:/sys/devices/cpu/format# cat umask
>   config:8-15
> 
> The valid bits (bit8-bit15) are recorded in bitmap of format 'umask'.
> 
> We collect total valid bits of all formats, save to a local variable
> 'masks' and reverse it. Now '~masks' represents total invalid bits.
> 
> bits = config & ~masks;
> 
> The set bits in 'bits' indicate the invalid bits used in config.
> Finally we use bitmap_scnprintf to report the invalid bits.
> 
> Some architectures may not export supported bits through sysfs,
> so if masks is 0, perf_pmu__warn_invalid_config directly returns.
> 
> After:
> 
> Single event without name:
> 
>   # ./perf stat -e cpu/r031234/ -a -- sleep 1
>   WARNING: event 'N/A' not valid (bits 16-17 of config '31234' not supported by kernel)!
> 
>    Performance counter stats for 'system wide':
> 
>                    0      cpu/r031234/
> 
>          1.001597373 seconds time elapsed
> 
> Multiple events with names:
> 
>   # ./perf stat -e cpu/rf01234,name=aaa/,cpu/r031234,name=bbb/ -a -- sleep 1
>   WARNING: event 'aaa' not valid (bits 20,22 of config 'f01234' not supported by kernel)!
>   WARNING: event 'bbb' not valid (bits 16-17 of config '31234' not supported by kernel)!
> 
>    Performance counter stats for 'system wide':
> 
>                    0      aaa
>                    0      bbb
> 
>          1.001573787 seconds time elapsed
> 
> Warnings are reported for invalid bits.
> 
> Co-developed-by: Jiri Olsa <jolsa@...hat.com>
> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>

Reviewed-by: Jiri Olsa <jolsa@...hat.com>

thanks,
jirka