linux-kernel - Re: [PATCH] KVM: x86: Add kvm_x86_ops hook to short circuit emulation

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: <20200916173416.GF10227@sjchrist-ice>
Date:   Wed, 16 Sep 2020 10:34:16 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Wanpeng Li <kernellwp@...il.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Tom Lendacky <thomas.lendacky@....com>
Subject: Re: [PATCH] KVM: x86: Add kvm_x86_ops hook to short circuit emulation

On Wed, Sep 16, 2020 at 09:31:22AM +0800, Wanpeng Li wrote:
> On Wed, 16 Sep 2020 at 07:29, Sean Christopherson
> <sean.j.christopherson@...el.com> wrote:
> >
> > Replace the existing kvm_x86_ops.need_emulation_on_page_fault() with a
> > more generic is_emulatable(), and unconditionally call the new function
> > in x86_emulate_instruction().
> >
> > KVM will use the generic hook to support multiple security related
> > technologies that prevent emulation in one way or another.  Similar to
> > the existing AMD #NPF case where emulation of the current instruction is
> > not possible due to lack of information, AMD's SEV-ES and Intel's SGX
> > and TDX will introduce scenarios where emulation is impossible due to
> > the guest's register state being inaccessible.  And again similar to the
> > existing #NPF case, emulation can be initiated by kvm_mmu_page_fault(),
> > i.e. outside of the control of vendor-specific code.
> >
> > While the cause and architecturally visible behavior of the various
> > cases are different, e.g. SGX will inject a #UD, AMD #NPF is a clean
> > resume or complete shutdown, and SEV-ES and TDX "return" an error, the
> > impact on the common emulation code is identical: KVM must stop
> > emulation immediately and resume the guest.
> >
> > Query is_emulatable() in handle_ud() as well so that the
> > force_emulation_prefix code doesn't incorrectly modify RIP before
> > calling emulate_instruction() in the absurdly unlikely scenario that
> > KVM encounters forced emulation in conjunction with "do not emulate".

...

> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 539ea1cd6020..5208217049d9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5707,6 +5707,9 @@ int handle_ud(struct kvm_vcpu *vcpu)
> >         char sig[5]; /* ud2; .ascii "kvm" */
> >         struct x86_exception e;
> >
> > +       if (unlikely(!kvm_x86_ops.is_emulatable(vcpu, NULL, 0)))
> > +               return 1;
> > +
> 
> Both VMX and SVM scenarios always fail this check.

Ah, right.  This patch was extracted from my SGX series, in which case there
would be a follow-up patch to add a VMX scenario where is_emulated() could
return false.

The intent of posting the patch standalone is so that SGX, SEV-ES, and/or TDX
have "ready to go" support in upstream, i.e. can change only the VMX/SVM
implementation of is_emulated().  I'm a-ok dropping the handle_ud() change,
or even the whole patch, until one of the above three is actually ready for
inclusion.