From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mark Kanda <mark.kanda@oracle.com>
Subject: Re: [PATCH] KVM: nVMX: Eliminate vmcs02 pool
Date: Mon, 27 Nov 2017 14:36:43 -0600
Message-ID: <f23dccdd-d368-e86f-2791-b1f4825343c5@oracle.com>
References: <e0e08f11f9d4a0c06e462a4a4217e996da058913.1511284598.git.mark.kanda@oracle.com>
 <da3c3271-3ece-db85-78c9-a3f7b4108c89@redhat.com>
 <0dd4862d-f1dd-bd08-a91d-97060c7b5f42@oracle.com>
 <3f88befc-a296-a8a7-f2d4-28afd15d632e@redhat.com>
 <e6978dbc-737a-df09-16bd-00fa01bfe287@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Cc: rkrcmar@redhat.com, ameya.more@oracle.com,
        Jim Mattson <jmattson@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from aserp1040.oracle.com ([141.146.126.69]:47805 "EHLO
        aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752609AbdK0Uf4 (ORCPT <rfc822;kvm@vger.kernel.org>);
        Mon, 27 Nov 2017 15:35:56 -0500
In-Reply-To: <e6978dbc-737a-df09-16bd-00fa01bfe287@oracle.com>
Content-Language: en-US
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 11/27/2017 2:04 PM, Mark Kanda wrote:
> On 11/27/2017 11:51 AM, Paolo Bonzini wrote:
>> On 27/11/2017 18:43, Mark Kanda wrote:
>>> On 11/23/2017 5:46 PM, Paolo Bonzini wrote:
>>>> On 21/11/2017 18:22, Mark Kanda wrote:
>>>>> -    nested_free_all_saved_vmcss(vmx);
>>>>> +    free_loaded_vmcs(&vmx->nested.vmcs02);
>>>>
>>>> Please add to free_loaded_vmcs a WARN that the passed vmcs is not
>>>> vmx->loaded_vmcs.
>>>
>>> Sure.
>>>
>>> However, I don't see a way to access vmx from the passed in vmcs, which
>>> would necessitate passing in vmx for the WARN (multiple callers) - I may
>>> be missing something..
>>
>> free_loaded_vmcs is only ever used on VMCS02's, so you can change it to
> 
> Perhaps I'm missing something, but it seems the free_loaded_vmcs() use 
> in vmx_create_vcpu() (and perhaps vmx_free_vcpu()) is for VMCS01 ..correct?
> 

How about I leave vmx_create_vcpu(), vmx_free_vcpu(), and 
free_loaded_vmcs() unmodified, and use the following for VMCS02 cases 
(in enter_vmx_operation() and free_nested()):

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);
}

..okay?

Thanks,

-Mark