Re: [PATCH v3] x86/speculation, KVM: only IBPB for switch_mm_always_ibpb on vCPU load

Jon Kohler
2022-05-12T17:58:10Z

> On May 12, 2022, at 9:44 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, May 10, 2022 at 03:50:31PM +0000, Sean Christopherson wrote:
>>> x86/speculation, KVM: remove IBPB on vCPU load
>>>
>>> Remove IBPB that is done on KVM vCPU load, as the guest-to-guest
>>> attack surface is already covered by switch_mm_irqs_off() ->
>>> cond_mitigation().
>>>
>>> The original 15d45071523d ("KVM/x86: Add IBPB support") was simply wrong in
>>> its guest-to-guest design intention. There are three scenarios at play
>>> here:
>>>
>>> 1. If the vCPUs belong to the same VM, they are in the same security
>>> domain and do not need an IPBP.
>>> 2. If the vCPUs belong to different VMs, and each VM is in its own mm_struct,
>>> switch_mm_irqs_off() will handle IBPB as an mm switch is guaranteed to
>>> occur prior to loading a vCPU belonging to a different VMs.
>>> 3. If the vCPUs belong to different VMs, but multiple VMs share an mm_struct,
>>> then the security benefits of an IBPB when switching vCPUs are dubious,
>>> at best.
>>>
>>> Issuing IBPB from KVM vCPU load would only cover #3, but there are no
>>
>> Just to hedge, there are no _known_ use cases.
>>
>>> real world tangible use cases for such a configuration.
>>
>> and I would further qualify this with:
>>
>> but there are no known real world, tangible use cases for running multiple
>> VMs belonging to different security domains in a shared address space.
>>
>> Running multiple VMs in a single address space is plausible and sane, _if_ they
>> are all in the same security domain or security is not a concern. That way the
>> statement isn't invalidated if someone pops up with a use case for running multiple
>> VMs but has no security story.
>>
>> Other than that, LGTM.
>>
>>> If multiple VMs
>>> are sharing an mm_structs, prediction attacks are the least of their
>>> security worries.
>>>
>>> Fixes: 15d45071523d ("KVM/x86: Add IBPB support")
>>> (Reviewedby/signed of by people here)
>>> (Code change simply whacks IBPB in KVM vmx/svm and thats it)
>
> I agree with all that I've read so far - the only thing that's missing is:
>
> (Documentation in Documentation/admin-guide/hw-vuln/spectre.rst about what the use
> cases are and what we're protecting against and what we're *not* protecting
> against because <raisins>).
>
> Thx.

Ok Thanks, Boris. I’ll review that doc and make modifications on v4, and make sure
that you are cc’d.

Thanks again,
Jon

>
> --
> Regards/Gruss,
> Boris.
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__people.kernel.org_tglx_notes-2Dabout-2Dnetiquette&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=55IDSpFE7N1d0eOYIL-UhgxoFg5JT7HFCEx17rNfo8XDAoJgj4xHjTzvqKec6Zi6&s=4ijrpeiLfGJRiyOpYY0Pn-BxvGEqvO2T7xaNyC0LmMk&e=