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

Jim Mattson
2020-10-15T02:14:50Z

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?