linux-kernel - Re: [PATCH 1/2] perf vendor events: Add stepping in CPUID string for x86

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, 15 Nov 2018 10:53:42 -0500
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     acme@...nel.org, mingo@...hat.com, peterz@...radead.org,
        linux-kernel@...r.kernel.org, namhyung@...nel.org,
        ak@...ux.intel.com
Subject: Re: [PATCH 1/2] perf vendor events: Add stepping in CPUID string for
 x86


>>>> +	/*
>>>> +	 * Full CPUID format is required to identify a platform.
>>>> +	 * Error out if the cpuid string is incomplete.
>>>> +	 */
>>>> +	if (full_mapcpuid && !full_cpuid) {
>>>> +		pr_info("Invalid CPUID %s. Full CPUID is required, "
>>>> +			"vendor-family-model-stepping\n", cpuid);
>>>> +		return 1;
>>>> +	}
>>>> +
>>>> +	if (regcomp(&re, mapcpuid, REG_EXTENDED) != 0) {
>>>> +		/* Warn unable to generate match particular string. */
>>>> +		pr_info("Invalid regular expression %s\n", mapcpuid);
>>>> +		return 1;
>>>> +	}
>>>> +
>>>> +	match = !regexec(&re, cpuid, 1, pmatch, 0);
>>>> +	regfree(&re);
>>>> +	if (match) {
>>>> +		size_t match_len = (pmatch[0].rm_eo - pmatch[0].rm_so);
>>>> +		size_t cpuid_len;
>>>> +
>>>> +		/* If the full CPUID format isn't required,
>>>> +		 * ignoring the stepping.
>>>> +		 */
>>>> +		if (!full_mapcpuid && full_cpuid)
>>>> +			cpuid_len = strrchr(cpuid, '-') - cpuid;
>>>> +		else
>>>> +			cpuid_len = strlen(cpuid);
>>>> +
>>>> +
>>>> +		/* Verify the entire string matched. */
>>>> +		if (match_len == cpuid_len)
>>>> +			return 0;
>>>
>>> why is this necessary?
>>>
>>
>> It's from previous common code. As my understanding, it just double check
>> the matched strings. There is no harmful. So I keep it.
> 
> right.. did you consider using the wildcard in the map file
> so it'd cover the stepping, having entries like:
> 
>    GenuineIntel-6-1F-*,v2,nehalemep,core
> 
> I haven't thought this one through, but seems we could bypass
> those '-stepping' checks.. but probably other changes would be
> necessary for the wildcard
> 

If using wildcard, the full cpuid format is required for all platforms.
The script which using environment string "PERF_CPUID" will definitely 
be broken on all platforms.
This patch (non-wildcard) only breaks the script on Skylake server and 
Cascadelake server.

I think that's the only drawback of using wildcard.

Thanks,
Kan