linux-kernel - Re: [PATCH 3.17 100/141] x86, microcode: Fix accessing dis_ucode

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:	Tue, 25 Nov 2014 16:59:34 -0500
From:	Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:	Borislav Petkov <bp@...e.de>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Richard Hendershot <rshendershot@...si.com>,
	David Vrabel <david.vrabel@...rix.com>
Subject: Re: [PATCH 3.17 100/141] x86, microcode: Fix accessing dis_ucode_ldr
 on 32-bit

On 11/25/2014 04:17 PM, Borislav Petkov wrote:
> On Tue, Nov 25, 2014 at 03:36:34PM -0500, Konrad Rzeszutek Wilk wrote:
>> Is there an use-case for this in virtualization at all?
> Not that I know of...
>
>> Why not make it in general then? Like:
>>
>> if (cpu_has_hypervisor)
>> 	return;
> Ah, good idea. Although we need to do it by-foot because the cpu_has
> stuff hasn't been initialized yet that early. Boris, I'm guessing
> something that should work... ?
>
> ---
> diff --git a/arch/x86/kernel/cpu/microcode/core_early.c b/arch/x86/kernel/cpu/microcode/core_early.c
> index 2c017f242a78..77137b317e2a 100644
> --- a/arch/x86/kernel/cpu/microcode/core_early.c
> +++ b/arch/x86/kernel/cpu/microcode/core_early.c
> @@ -74,6 +74,16 @@ static int x86_family(void)
>   	return x86;
>   }
>   
> +static bool x86_guest(void)
> +{
> +	u32 eax = 0x1;
> +	u32 ebx, ecx = 0, edx;
> +
> +	native_cpuid(&eax, &ebx, &ecx, &edx);

This should be cpuid(0x1, &eax, &ebx, &ecx, &edx). Otherwise we are not 
getting bits that the hypervisor wants the guest to see (on Xen cpuid() 
turns into hypercall, on baremetal it's native).

With that change it works and

Tested-by: Boris Ostrovsky <boris.ostrovsky@...cle.com>

(May be worth adding a comment as to what is_guest() is checking for 
since 31 is a magic number).

BTW, the crash had nothing to do with accessing dis_ucode_ldr, we are 
crashing much later, in load_ucode_intel_ap(), trying to access 
*initrd_start_p. And the reason we didn't crash before was because 
compiler optimized out whole load_ucode_ap() since 
check_loader_disabled_ap() was always true.

Thanks.
-boris

> +
> +	return !!(ecx & BIT(31));
> +}
> +
>   static bool __init check_loader_disabled_bsp(void)
>   {
>   #ifdef CONFIG_X86_32
> @@ -98,6 +108,9 @@ void __init load_ucode_bsp(void)
>   {
>   	int vendor, x86;
>   
> +	if (x86_guest())
> +		return;
> +
>   	if (check_loader_disabled_bsp())
>   		return;
>   
> @@ -134,6 +147,9 @@ void load_ucode_ap(void)
>   {
>   	int vendor, x86;
>   
> +	if (x86_guest())
> +		return;
> +
>   	if (check_loader_disabled_ap())
>   		return;
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/