linux-kernel - Re: [PATCH v3 8/8] KVM: nSVM: implement ondemand allocation of the nested state

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:   Sun, 13 Sep 2020 22:57:38 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@...r.kernel.org>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Jim Mattson <jmattson@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <joro@...tes.org>,
        Wanpeng Li <wanpengli@...cent.com>
Subject: Re: [PATCH v3 8/8] KVM: nSVM: implement ondemand allocation of the
 nested state

On Sat, 2020-09-12 at 18:32 +0200, Paolo Bonzini wrote:
> On 27/08/20 19:11, Maxim Levitsky wrote:
> > +	hsave_page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
> > +	if (!hsave_page)
> > +		goto error;
> > +
> 
> I think an error here should be just an internal error userspace exit,
> or a -ENOMEM from KVM_RUN; not a #GP in the guest[1].  However, that's
> difficult to plug into KVM.  Can you instead allocate nested state if
> KVM_SET_CPUID2 sets the SVM bit?  Returning -ENOMEM from KVM_SET_CPUID2
> is more likely to be something that userspace copes with.

This would be a bit sad thing to do, as it would allocate nested state for each
guest where it is enabled in CPUID, which is IMHO already the default or soon to be.

Currently nested KVM is nice to only enable EFER.SVME when a vm is created (in svm_hardware_enable)
which means that with this on demand nested state allocation, we only use nested state
for guests that actually use nested as opposed to be merely enabled.

About injecting the #GP,
I probably can make the guest triple fault instead if we consider this to be better solution.
in terms of not allowing to continue guest execution.
I somehow thought that guest will already be killed on emulation failure,
but now I do see that kvm_emulate_wrmsr and that it injects a #GP

What do you think?

> 
> I queued patches 1-5, and 7 for 5.9-rc.

Thanks!

Best regards,
	Maxim Levitsky

> 
> Paolo
> 
> [1] Though in practice an order 0 allocation will never fail
>