On 09/07/2016 00:36, Jim Mattson wrote:
> When freeing the nested resources of a vcpu, there is an assumption that
> the vcpu's vmcs01 is the current VMCS on the CPU that executes
> nested_release_vmcs12(). If this assumption is violated, the vcpu's
> vmcs01 may be made active on multiple CPUs at the same time, in
> violation of Intel's specification. Moreover, since the vcpu's vmcs01 is
> not VMCLEARed on every CPU on which it is active, it can linger in a
> CPU's VMCS cache after it has been freed and potentially
> repurposed. Subsequent eviction from the CPU's VMCS cache on a capacity
> miss can result in memory corruption.
>
> It is not sufficient for vmx_free_vcpu() to call vmx_load_vmcs01(). If
> the vcpu in question was last loaded on a different CPU, it must be
> migrated to the current CPU before calling vmx_load_vmcs01().
>
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
>
> ---
> arch/x86/kvm/vmx.c | 19 +++++++++++++++++--
> virt/kvm/kvm_main.c | 2 ++
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 003618e..afb1ab3 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8855,6 +8855,22 @@ static void vmx_load_vmcs01(struct kvm_vcpu *vcpu)
> put_cpu();
> }
>
> +/*
> + * Ensure that the current vmcs of the logical processor is the
> + * vmcs01 of the vcpu before calling free_nested().
> + */
> +static void vmx_free_vcpu_nested(struct kvm_vcpu *vcpu)
> +{
> + struct vcpu_vmx *vmx = to_vmx(vcpu);
> + int r;
> +
> + r = vcpu_load(vcpu);
> + BUG_ON(r);
> + vmx_load_vmcs01(vcpu);
> + free_nested(vmx);
> + vcpu_put(vcpu);
> +}
> +
> static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -8863,8 +8879,7 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
> vmx_destroy_pml_buffer(vmx);
> free_vpid(vmx->vpid);
> leave_guest_mode(vcpu);
> - vmx_load_vmcs01(vcpu);
> - free_nested(vmx);
> + vmx_free_vcpu_nested(vcpu);
> free_loaded_vmcs(vmx->loaded_vmcs);
> kfree(vmx->guest_msrs);
> kvm_vcpu_uninit(vcpu);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 48bd520..dd25346 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -148,6 +148,7 @@ int vcpu_load(struct kvm_vcpu *vcpu)
> put_cpu();
> return 0;
> }
> +EXPORT_SYMBOL_GPL(vcpu_load);
>
> void vcpu_put(struct kvm_vcpu *vcpu)
> {
> @@ -157,6 +158,7 @@ void vcpu_put(struct kvm_vcpu *vcpu)
> preempt_enable();
> mutex_unlock(&vcpu->mutex);
> }
> +EXPORT_SYMBOL_GPL(vcpu_put);
>
> static void ack_flush(void *_completed)
> {
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Looks good, thanks! I haven't yet pushed it out, but I plan to send it
to Linus for 4.7. So also
Cc: stable <stable@xxxxxxxxxxxxxxx>
Thanks,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
