On Thu, Sep 12, 2019 at 1:51 AM Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: > > Fuqian Huang <huangfq.daxian@xxxxxxxxx> writes: > > > Emulation of VMPTRST can incorrectly inject a page fault > > when passed an operand that points to an MMIO address. > > The page fault will use uninitialized kernel stack memory > > as the CR2 and error code. > > > > The right behavior would be to abort the VM with a KVM_EXIT_INTERNAL_ERROR > > exit to userspace; > > Hm, why so? KVM_EXIT_INTERNAL_ERROR is basically an error in KVM, this > is not a proper reaction to a userspace-induced condition (or ever). This *is* an error in KVM. KVM should properly emulate the quadword store to the emulated device. Doing anything else is just wrong. KVM_INTERNAL_ERROR is basically a cop-out for things that are hard. > I also looked at VMPTRST's description in Intel's manual and I can't > find and explicit limitation like "this must be normal memory". We're > just supposed to inject #PF "If a page fault occurs in accessing the > memory destination operand." > > In case it seems to be too cumbersome to handle VMPTRST to MMIO and we > think that nobody should be doing that I'd rather prefer injecting #GP. That is not the architected behavior at all. Now you're just making things up! > Please tell me what I'm missing :-) > > > however, it is not an easy fix, so for now just ensure > > that the error code and CR2 are zero. > > > > Signed-off-by: Fuqian Huang <huangfq.daxian@xxxxxxxxx> > > --- > > arch/x86/kvm/x86.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 290c3c3efb87..7f442d710858 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -5312,6 +5312,7 @@ int kvm_write_guest_virt_system(struct kvm_vcpu *vcpu, gva_t addr, void *val, > > /* kvm_write_guest_virt_system can pull in tons of pages. */ > > vcpu->arch.l1tf_flush_l1d = true; > > > > + memset(exception, 0, sizeof(*exception)); > > return kvm_write_guest_virt_helper(addr, val, bytes, vcpu, > > PFERR_WRITE_MASK, exception); > > } > > -- > Vitaly
