linux-kernel - Re: [PATCH] x86/cpuid: Deal with broken firmware once more

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: <9badae12-f07e-b2e3-c1ce-c2017cd5e510@brocade.com>
Date:   Wed, 9 Nov 2016 11:34:52 -0500
From:   "Charles (Chas) Williams" <ciwillia@...cade.com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>
CC:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        "M. Vefa Bicakci" <m.v.b@...box.com>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] x86/cpuid: Deal with broken firmware once more



On 11/09/2016 11:03 AM, Peter Zijlstra wrote:
> On Wed, Nov 09, 2016 at 04:35:51PM +0100, Thomas Gleixner wrote:
>> Both ACPI and MP specifications require that the APIC id in the respective
>> tables must be the same as the APIC id in CPUID.
>>
>> The kernel retrieves the physical package id from the APIC id during the
>> ACPI/MP table scan and builds the physical to logical package map.
>>
>> There exist Virtualbox and Xen implementations which violate the spec. As a
>
> ISTR it was VMware, not VirtualBox, but whatever.. they're both crazy
> virt stuff.

Yes, this was VMware in particular.  It would be good to get this comment
right so as not to mislead anyone.

>>  /*
>> + * The physical to logical package id mapping is initialized from the
>> + * acpi/mptables information. Make sure that CPUID actually agrees with
>> + * that.
>> + */
>> +static void sanitize_package_id(struct cpuinfo_x86 *c)
>> +{
>> +#ifdef CONFIG_SMP
>> +	unsigned int pkg, apicid, cpu = smp_processor_id();
>> +
>> +	apicid = apic->cpu_present_to_apicid(cpu);
>> +	pkg = apicid >> boot_cpu_data.x86_coreid_bits;
>> +
>> +	if (apicid != c->initial_apicid) {
>> +		pr_err(FW_BUG "CPU%u: APIC id mismatch. Firmware: %x CPUID: %x\n",
>> +		       cpu, apicid, c->initial_apicid);
>
> Should we not also 'fix' c->initial_apicid ?

Since we have c->apicid and c->initial_apicid it seems reasonable to keep one
set to the "correct" value.  I don't think c->initial_apicid is used past
this.

I should have some tests on this patch later today.