linux-kernel - Re: Can't boot as Xen dom0 due to commit fe055896

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: <51f0a58d-a5bc-aae4-22b2-f45bf7164088@oracle.com>
Date:   Thu, 15 Dec 2016 17:56:44 -0500
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Borislav Petkov <bp@...e.de>
Cc:     Juergen Gross <jgross@...e.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        xen-devel <xen-devel@...ts.xenproject.org>
Subject: Re: Can't boot as Xen dom0 due to commit fe055896

On 12/15/2016 03:03 PM, Borislav Petkov wrote:
> On Thu, Dec 15, 2016 at 02:36:46PM -0500, Boris Ostrovsky wrote:
>> We are calling find_proper_container(..., &eq_id) and determine result
>> based on eq_id not being zero. If find_proper_container() doesn't find
>> anything it will not modify eq_id and so you get back whatever you
>> passed in.
> The correct fix for that is to not return struct container at all from
> those functions but simply an eq_id or even an int error value or so.
> Passing back a struct is simply silly. I'll prep a proper patch for
> that.
>
>> The fact that  I am having problems with HVM may or may not have
>> anything to do with microcode. I don't know yet but it's separate from
>> save_microcode_in_initrd_amd() patch. I am pretty sure about that
>> because unlike PV it is failing in early boot code.

For AMD you also need something like:

--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -297,6 +297,7 @@ void __init load_ucode_amd_bsp(unsigned int family)
        struct cpio_data cp;
        const char *path;
        bool use_pa;
+       u32 eax = 1, ebx, ecx = 0, edx;
 
        if (IS_ENABLED(CONFIG_X86_32)) {
                uci     = (struct ucode_cpu_info
*)__pa_nodebug(ucode_cpu_info);
@@ -315,7 +316,8 @@ void __init load_ucode_amd_bsp(unsigned int family)
                return;
 
        /* Get BSP's CPUID.EAX(1), needed in load_microcode_amd() */
-       uci->cpu_sig.sig = cpuid_eax(1);
+       native_cpuid(&eax, &ebx, &ecx, &edx);
+       uci->cpu_sig.sig = eax;
 
        apply_microcode_early_amd(cp.data, cp.size, true);
 }


With CONFIG_PARAVIRT (which I suspect you don't have on) cpuid() is a
call via a pointer, you can see it, for example, if you disassemble
load_ucode_amd_bsp(). And we don't have paging on yet when we call
load_ucode_amd_bsp (at least in 32-bit mode).


> I still need a fix for Jürgen's use case where he lands in the
> microcode loader. And that he shouldn't do in the first place when
> running in a xen guest.
>

Since this happens during AP bringup PV cpuid is available and can be
checked to see whether we are a guest.

But I think it may be worthwhile trying to understand why we are dying,
it hasn't happened until now.

-boris