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@xxxxxxxxxx>
> Signed-off-by: Mark Kanda <mark.kanda@xxxxxxxxxx>
> Reviewed-by: Ameya More <ameya.more@xxxxxxxxxx>
> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
--
Thanks,
David / dhildenb
