Re: [PATCH v3 7/9] KVM: VMX: Add guest physical address check in EPT violation and misconfig

[Jim Mattson <jmattson@google.com>]

On Fri, Oct 9, 2020 at 9:17 AM Jim Mattson <jmattson@google.com> wrote:
>
> On Fri, Jul 10, 2020 at 8:48 AM Mohammed Gamal <mgamal@redhat.com> wrote:
> >
> > Check guest physical address against it's maximum physical memory. If
> > the guest's physical address exceeds the maximum (i.e. has reserved bits
> > set), inject a guest page fault with PFERR_RSVD_MASK set.
> >
> > This has to be done both in the EPT violation and page fault paths, as
> > there are complications in both cases with respect to the computation
> > of the correct error code.
> >
> > For EPT violations, unfortunately the only possibility is to emulate,
> > because the access type in the exit qualification might refer to an
> > access to a paging structure, rather than to the access performed by
> > the program.
> >
> > Trapping page faults instead is needed in order to correct the error code,
> > but the access type can be obtained from the original error code and
> > passed to gva_to_gpa.  The corrections required in the error code are
> > subtle. For example, imagine that a PTE for a supervisor page has a reserved
> > bit set.  On a supervisor-mode access, the EPT violation path would trigger.
> > However, on a user-mode access, the processor will not notice the reserved
> > bit and not include PFERR_RSVD_MASK in the error code.
> >
> > Co-developed-by: Mohammed Gamal <mgamal@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/kvm/vmx/vmx.c | 24 +++++++++++++++++++++---
> >  arch/x86/kvm/vmx/vmx.h |  3 ++-
> >  2 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 770b090969fb..de3f436b2d32 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -4790,9 +4790,15 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
> >
> >         if (is_page_fault(intr_info)) {
> >                 cr2 = vmx_get_exit_qual(vcpu);
> > -               /* EPT won't cause page fault directly */
> > -               WARN_ON_ONCE(!vcpu->arch.apf.host_apf_flags && enable_ept);
> > -               return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
> > +               if (enable_ept && !vcpu->arch.apf.host_apf_flags) {
> > +                       /*
> > +                        * EPT will cause page fault only if we need to
> > +                        * detect illegal GPAs.
> > +                        */
> > +                       kvm_fixup_and_inject_pf_error(vcpu, cr2, error_code);
> > +                       return 1;
> > +               } else
> > +                       return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
> >         }
> >
> >         ex_no = intr_info & INTR_INFO_VECTOR_MASK;
> > @@ -5308,6 +5314,18 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
> >                PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
> >
> >         vcpu->arch.exit_qualification = exit_qualification;
> > +
> > +       /*
> > +        * Check that the GPA doesn't exceed physical memory limits, as that is
> > +        * a guest page fault.  We have to emulate the instruction here, because
> > +        * if the illegal address is that of a paging structure, then
> > +        * EPT_VIOLATION_ACC_WRITE bit is set.  Alternatively, if supported we
> > +        * would also use advanced VM-exit information for EPT violations to
> > +        * reconstruct the page fault error code.
> > +        */
> > +       if (unlikely(kvm_mmu_is_illegal_gpa(vcpu, gpa)))
> > +               return kvm_emulate_instruction(vcpu, 0);
> > +
>
> Is kvm's in-kernel emulator up to the task? What if the instruction in
> question is AVX-512, or one of the myriad instructions that the
> in-kernel emulator can't handle? Ice Lake must support the advanced
> VM-exit information for EPT violations, so that would seem like a
> better choice.
>
Anyone?