| 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
| ||
|
Message-ID: <abdfe6de-8338-6e64-0e6c-a387aed725e6@redhat.com>
Date: Thu, 18 Jan 2018 16:32:15 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>,
David Woodhouse <dwmw2@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Josh Poimboeuf <jpoimboe@...hat.com>
Cc: linux-kernel@...r.kernel.org, Dave Hansen <dave.hansen@...el.com>,
Ashok Raj <ashok.raj@...el.com>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Andy Lutomirski <luto@...nel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Greg KH <gregkh@...uxfoundation.org>,
Andrea Arcangeli <aarcange@...hat.com>,
Andi Kleen <ak@...ux.intel.com>,
Arjan Van De Ven <arjan.van.de.ven@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Jun Nakajima <jun.nakajima@...el.com>,
Asit Mallick <asit.k.mallick@...el.com>,
Jason Baron <jbaron@...mai.com>,
David Woodhouse <dwmw@...zon.co.uk>
Subject: Re: [PATCH 34/35] x86/kvm: Add IBPB support
On 18/01/2018 14:48, Peter Zijlstra wrote:
> From: Ashok Raj <ashok.raj@...el.com>
>
> Add MSR passthrough for MSR_IA32_PRED_CMD and place branch predictor
> barriers on switching between VMs to avoid inter VM specte-v2 attacks.
>
> [peterz: rebase and changelog rewrite]
>
> Cc: Asit Mallick <asit.k.mallick@...el.com>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Arjan Van De Ven <arjan.van.de.ven@...el.com>
> Cc: Tim Chen <tim.c.chen@...ux.intel.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Andrea Arcangeli <aarcange@...hat.com>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Jun Nakajima <jun.nakajima@...el.com>
> Cc: Andy Lutomirski <luto@...nel.org>
> Cc: Greg KH <gregkh@...uxfoundation.org>
> Cc: David Woodhouse <dwmw@...zon.co.uk>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Signed-off-by: Ashok Raj <ashok.raj@...el.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
> arch/x86/kvm/svm.c | 8 ++++++++
> arch/x86/kvm/vmx.c | 8 ++++++++
> 2 files changed, 16 insertions(+)
This patch is missing the AMD-specific CPUID bit.
In addition, it is not bisectable because guests will see the SPEC_CTRL CPUID
bit after patch 32 even if PRED_CMD is not supported yet.
The simplest solutions are:
1) add the indirect_branch_prediction_barrier() calls first, and squash
everything else including the AMD CPUID bit into a single patch.
2) place the IBPB in this series, and only add stop/restart_indirect_branch_
speculation() to the vmexit and vmentry paths. We will do the guest enabling
in kvm.git after this is ready, it should only take a week and we'll ensure
it is backportable to 4.14 and 4.15.
3) same as (2) except we'll send the guest enabling through TIP.
Paolo
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -252,6 +252,7 @@ static const struct svm_direct_access_ms
> { .index = MSR_SYSCALL_MASK, .always = true },
> #endif
> { .index = MSR_IA32_SPEC_CTRL, .always = true },
> + { .index = MSR_IA32_PRED_CMD, .always = true },
> { .index = MSR_IA32_LASTBRANCHFROMIP, .always = false },
> { .index = MSR_IA32_LASTBRANCHTOIP, .always = false },
> { .index = MSR_IA32_LASTINTFROMIP, .always = false },
> @@ -532,6 +533,7 @@ struct svm_cpu_data {
> struct kvm_ldttss_desc *tss_desc;
>
> struct page *save_area;
> + struct vmcb *current_vmcb;
> };
>
> static DEFINE_PER_CPU(struct svm_cpu_data *, svm_data);
> @@ -1709,11 +1711,13 @@ static void svm_free_vcpu(struct kvm_vcp
> __free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
> kvm_vcpu_uninit(vcpu);
> kmem_cache_free(kvm_vcpu_cache, svm);
> + indirect_branch_prediction_barrier();
> }
>
> static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> + struct svm_cpu_data *sd = per_cpu(svm_data, cpu);
> int i;
>
> if (unlikely(cpu != vcpu->cpu)) {
> @@ -1742,6 +1746,10 @@ static void svm_vcpu_load(struct kvm_vcp
> if (static_cpu_has(X86_FEATURE_RDTSCP))
> wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
>
> + if (sd->current_vmcb != svm->vmcb) {
> + sd->current_vmcb = svm->vmcb;
> + indirect_branch_prediction_barrier();
> + }
> avic_vcpu_load(vcpu, cpu);
> }
>
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2280,6 +2280,7 @@ static void vmx_vcpu_load(struct kvm_vcp
> if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) {
> per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
> vmcs_load(vmx->loaded_vmcs->vmcs);
> + indirect_branch_prediction_barrier();
> }
>
> if (!already_loaded) {
> @@ -3837,6 +3838,11 @@ static void free_loaded_vmcs(struct load
> free_vmcs(loaded_vmcs->vmcs);
> loaded_vmcs->vmcs = NULL;
> WARN_ON(loaded_vmcs->shadow_vmcs != NULL);
> + /*
> + * The VMCS could be recycled, causing a false negative in vmx_vcpu_load
> + * block speculative execution.
> + */
> + indirect_branch_prediction_barrier();
This IBPB is not needed, as loaded_vmcs->NULL is now NULL and there will be a
barrier the next time vmx_vcpu_load is called on this CPU.
Thanks,
Paolo
> }
>
> static void free_kvm_area(void)
> @@ -6804,6 +6810,8 @@ static __init int hardware_setup(void)
> */
> if (boot_cpu_has(X86_FEATURE_SPEC_CTRL))
> vmx_disable_intercept_for_msr(MSR_IA32_SPEC_CTRL, false);
> + if (boot_cpu_has(X86_FEATURE_IBPB))
> + vmx_disable_intercept_for_msr(MSR_IA32_PRED_CMD, false);
>
> vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
>
>
Powered by blists - more mailing lists