linux-kernel - Re: [PATCH v2 3/7] KVM: nVMX: Ignore 'hv_clean_fields' data when eVMCS data is copied in vmx_get_nested

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 26 May 2021 17:44:23 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v2 3/7] KVM: nVMX: Ignore 'hv_clean_fields' data when
 eVMCS data is copied in vmx_get_nested_state()

On Mon, 2021-05-24 at 15:58 +0200, Paolo Bonzini wrote:
> On 24/05/21 15:01, Vitaly Kuznetsov wrote:
> > With 'need_vmcs12_to_shadow_sync', we treat eVMCS as shadow VMCS which
> > happens to shadow all fields and while it may not be the most optimal
> > solution, it is at least easy to comprehend. We can try drafting
> > something up instead, maybe it will also be good but honestly I'm afraid
> > of incompatible changes in KVM_GET_NESTED_STATE/KVM_SET_NESTED_STATE, we
> > can ask Paolo's opinion on that.
> 
> Yes, it's much easier to understand it if the eVMCS is essentially a 
> memory-backed shadow VMCS, than if it's really the vmcs12 format.  I 
> understand that it's bound to be a little slower, but at least the two 
> formats are not all over the place.
> 
> Paolo
> 

Hi!

Please see my other reply to this in patch 1.

I understand this concern, but what bugs me is that we sort of 
shouldn't read evmcs while L1 is running.
(e.g its clean bits might be not up to date and
such).

Actually instead of thinking of evmcs as a shadow, I am thinking of it
more as a vmcb12 (the SVM one), 
which we load when we do a nested entry and which is then
updated when we do a nested vmexit, and other than that, while
L1 is running, we don't touch it.

Yes there is that vm instruction error field in evmcs which I suppose we should
write when we fail a VMX instruction (invept only practically I think) 
while we just run L1, and even that we might just avoid doing, 
which will allow us to avoid even keeping
the evmcs mapped while L1 is running.

Just my 0.2 cents.

Best regards,
	Maxim Levitsky