linux-kernel - Re: [PATCH 6/7] x86/svm: Set IBPB when running a different VCPU

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: <aa2fcc44-15bf-d9a5-3831-76f6434bbaa3@redhat.com>
Date:   Tue, 9 Jan 2018 22:56:07 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc:     Arjan van de Ven <arjan@...ux.intel.com>,
        Liran Alon <liran.alon@...cle.com>, jmattson@...gle.com,
        dwmw@...zon.co.uk, bp@...en8.de, aliguori@...zon.com,
        thomas.lendacky@....com, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH 6/7] x86/svm: Set IBPB when running a different VCPU

On 09/01/2018 21:39, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 09, 2018 at 05:49:08PM +0100, Paolo Bonzini wrote:
>> On 09/01/2018 17:23, Arjan van de Ven wrote:
>>> On 1/9/2018 8:17 AM, Paolo Bonzini wrote:
>>>> On 09/01/2018 16:19, Arjan van de Ven wrote:
>>>>> On 1/9/2018 7:00 AM, Liran Alon wrote:
>>>>>>
>>>>>> ----- arjan@...ux.intel.com wrote:
>>>>>>
>>>>>>> On 1/9/2018 3:41 AM, Paolo Bonzini wrote:
>>>>>>>> The above ("IBRS simply disables the indirect branch predictor")
>>>>>>>> was my
>>>>>>>> take-away message from private discussion with Intel.  My guess is
>>>>>>>> that
>>>>>>>> the vendors are just handwaving a spec that doesn't match what
>>>>>>>> they have
>>>>>>>> implemented, because honestly a microcode update is unlikely to do
>>>>>>>> much
>>>>>>>> more than an old-fashioned chicken bit.  Maybe on Skylake it does
>>>>>>>> though, since the performance characteristics of IBRS are so
>>>>>>>> different
>>>>>>>> from previous processors.  Let's ask Arjan who might have more
>>>>>>>> information about it, and hope he actually can disclose it...
>>>>>>>
>>>>>>> IBRS will ensure that, when set after the ring transition, no earlier
>>>>>>> branch prediction data is used for indirect branches while IBRS is
>>>>>>> set
>>>>
>>>> Let me ask you my questions, which are independent of L0/L1/L2
>>>> terminology.
>>>>
>>>> 1) Is vmentry/vmexit considered a ring transition, even if the guest is
>>>> running in ring 0?  If IBRS=1 in the guest and the host is using IBRS,
>>>> the host will not do a wrmsr on exit.  Is this safe for the host kernel?
>>>
>>> I think the CPU folks would want us to write the msr again.
>>
>> Want us, or need us---and if we don't do that, what happens?  And if we
>> have to do it, how is IBRS=1 different from an IBPB?...
> 
> Arjan says 'ring transition' but I am pretty sure it is more of 'prediction
> mode change'. And from what I have gathered so far moving from lower (guest)
> to higher (hypervisor) has no bearing on the branch predicator. Meaning
> the guest ring0 can attack us if we don't touch this MSR.
> 
> We have to WRMSR 0x48 to 1 to flush out lower prediction. Aka this is a
> 'reset' button and at every 'prediction mode' you have to hit this.

That however makes me wonder why Intel said "before transitioning to
ring 3, do WRMSR to IA32_SPEC_CTRL to clear IBRS to 0".

I have looked again at the slides I had and "IBRS all the time" seems to
require an IBPB anyway, thus making me wonder what's the point of it at
all.  Why can't we have proper indexing of the BTB by PCID and VPID, and
forget IBRS completely on newer machines?!?

> <sigh> Can we have a discussion on making an kvm-security mailing list
> where we can figure all this out during embargo and not have these
> misunderstandings.

Being told who knows what from other companies, would also have been a
start.  Instead CVE-2017-5715 was disclosed to each partner
individually, and now _we_ are reaping what Intel has sown.

Paolo