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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 27 Sep 2018 15:44:32 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Jim Mattson <jmattson@...gle.com>,
        Liran Alon <liran.alon@...cle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 7/9] x86/kvm/nVMX: introduce source data cache for kvm_init_shadow_ept_mmu()

Vitaly Kuznetsov <vkuznets@...hat.com> writes:

> Sean Christopherson <sean.j.christopherson@...el.com> writes:
>
>> On Tue, Sep 25, 2018 at 07:58:42PM +0200, Vitaly Kuznetsov wrote:
...
>>>  
>>> -static union kvm_mmu_page_role
>>> -kvm_calc_shadow_ept_root_page_role(struct kvm_vcpu *vcpu, bool accessed_dirty)
>>> +static union kvm_mmu_role
>>> +kvm_calc_shadow_ept_root_page_role(struct kvm_vcpu *vcpu, bool accessed_dirty,
>>> +				   bool execonly)
>>>  {
>>> -	union kvm_mmu_page_role role = vcpu->arch.mmu->mmu_role.base;
>>> +	union kvm_mmu_role role = kvm_calc_mmu_role_common(vcpu);
>>
>> kvm_calc_mmu_role_common() doesn't preserve the current mmu_role.base
>> and kvm_calc_mmu_role_common() doesn't capture all base fields.  Won't
>> @role will be incorrect for base fields that aren't set below, e.g.
>> cr4_pae, smep_andnot_wp, smap_andnot_wp, etc...
>
> Oh, I see what you mean. Actually, PATCH8 of this series adds some of
> this stuff but smep_andnot_wp and smap_andnot_wp are still not set. I
> think I'll enhance kvm_calc_mmu_role_common() and move some stuff from
> PATCH8 to this one.
> (The fact that @role is currently not fully re-initialized here is very
> unobvious so I would definitely prefer to explicitly initialize
> everything over inheriting something from previously initialized role).

On the other hand if we want to perform full re-initialization we'll
have to distinguish between shadow and TDP here and this isn't what we
want. I'm about to change my mind as it seems that inheriting base role
here is not the worst idea after all...

-- 
Vitaly

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ