Re: [PATCH v2 2/2] KVM: nVMX: Add a WARN for freeing a loaded VMCS02

[David Hildenbrand <david@redhat.com>]

On 28.11.2017 00:22, Mark Kanda wrote:
> When attempting to free a loaded VMCS02, add a WARN and avoid
> freeing it (to avoid use-after-free situations).
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> Reviewed-by: Ameya More <ameya.more@oracle.com>
> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
>  arch/x86/kvm/vmx.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f348445..5b2cc5f 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3843,6 +3843,19 @@ static void free_loaded_vmcs(struct loaded_vmcs *loaded_vmcs)
>  	WARN_ON(loaded_vmcs->shadow_vmcs != NULL);
>  }
>  
> +static void vmx_nested_free_vmcs02(struct vcpu_vmx *vmx)
> +{
> +	struct loaded_vmcs *loaded_vmcs = &vmx->nested.vmcs02;
> +
> +	/*
> +	 * Just leak the VMCS02 if the WARN triggers. Better than
> +	 * a use-after-free.
> +	 */
> +	if (WARN_ON(vmx->loaded_vmcs == loaded_vmcs))
> +		return;
> +	free_loaded_vmcs(loaded_vmcs);
> +}
> +
>  static void free_kvm_area(void)
>  {
>  	int cpu;
> @@ -7187,7 +7200,7 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
>  	free_page((unsigned long)vmx->nested.msr_bitmap);
>  
>  out_msr_bitmap:
> -	free_loaded_vmcs(&vmx->nested.vmcs02);
> +	vmx_nested_free_vmcs02(vmx);
>  
>  out_vmcs02:
>  	return -ENOMEM;
> @@ -7358,7 +7371,7 @@ static void free_nested(struct vcpu_vmx *vmx)
>  		vmx->nested.pi_desc = NULL;
>  	}
>  
> -	free_loaded_vmcs(&vmx->nested.vmcs02);
> +	vmx_nested_free_vmcs02(vmx);
>  }
>  
>  /* Emulate the VMXOFF instruction */
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb