linux-kernel - Re: [RFC 02/16] x86/kvm: Introduce KVM memory protection feature

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]

Message-ID: <0cd53be8abede7e82a68c32b1d8b0e4ca6f24a05.camel@intel.com>
Date:   Wed, 3 Jun 2020 02:09:51 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "kirill@...temov.name" <kirill@...temov.name>,
        "Christopherson, Sean J" <sean.j.christopherson@...el.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "wad@...omium.org" <wad@...omium.org>,
        "Kleen, Andi" <andi.kleen@...el.com>,
        "luto@...nel.org" <luto@...nel.org>,
        "aarcange@...hat.com" <aarcange@...hat.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "wanpengli@...cent.com" <wanpengli@...cent.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "joro@...tes.org" <joro@...tes.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "jmattson@...gle.com" <jmattson@...gle.com>,
        "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [RFC 02/16] x86/kvm: Introduce KVM memory protection feature

On Wed, 2020-05-27 at 10:39 +0200, Vitaly Kuznetsov wrote:
> Sean Christopherson <sean.j.christopherson@...el.com> writes:
> 
> > On Mon, May 25, 2020 at 06:15:25PM +0300, Kirill A. Shutemov wrote:
> > > On Mon, May 25, 2020 at 04:58:51PM +0200, Vitaly Kuznetsov wrote:
> > > > > @@ -727,6 +734,15 @@ static void __init kvm_init_platform(void)
> > > > >  {
> > > > >  	kvmclock_init();
> > > > >  	x86_platform.apic_post_init = kvm_apic_init;
> > > > > +
> > > > > +	if (kvm_para_has_feature(KVM_FEATURE_MEM_PROTECTED)) {
> > > > > +		if (kvm_hypercall0(KVM_HC_ENABLE_MEM_PROTECTED)) {
> > > > > +			pr_err("Failed to enable KVM memory
> > > > > protection\n");
> > > > > +			return;
> > > > > +		}
> > > > > +
> > > > > +		mem_protected = true;
> > > > > +	}
> > > > >  }
> > > > 
> > > > Personally, I'd prefer to do this via setting a bit in a KVM-specific
> > > > MSR instead. The benefit is that the guest doesn't need to remember if
> > > > it enabled the feature or not, it can always read the config msr. May
> > > > come handy for e.g. kexec/kdump.
> > > 
> > > I think we would need to remember it anyway. Accessing MSR is somewhat
> > > expensive. But, okay, I can rework it MSR if needed.
> > 
> > I think Vitaly is talking about the case where the kernel can't easily get
> > at its cached state, e.g. after booting into a new kernel.  The kernel would
> > still have an X86_FEATURE bit or whatever, providing a virtual MSR would be
> > purely for rare slow paths.
> > 
> > That being said, a hypercall plus CPUID bit might be better, e.g. that'd
> > allow the guest to query the state without risking a #GP.
> 
> We have rdmsr_safe() for that! :-) MSR (and hypercall to that matter)
> should have an associated CPUID feature bit of course.
> 
> Yes, hypercall + CPUID would do but normally we treat CPUID data as
> static and in this case we'll make it a dynamically flipping
> bit. Especially if we introduce 'KVM_HC_DISABLE_MEM_PROTECTED' later.

Not sure why is KVM_HC_DISABLE_MEM_PROTECTED needed?

> 
> > > Note, that we can avoid the enabling algother, if we modify BIOS to deal
> > > with private/shared memory. Currently BIOS get system crash if we enable
> > > the feature from time zero.
> > 
> > Which would mesh better with a CPUID feature bit.
> > 
> 
> And maybe even help us to resolve 'reboot' problem.

IMO we can ask Qemu to call hypercall to 'enable' memory protection when
creating VM, and guest kernel *queries* whether it is protected via CPUID
feature bit.