On Fri, Aug 09, 2024 at 12:02:59PM -0700, Sean Christopherson wrote:
> Set PFERR_GUEST_{FINAL,PAGE}_MASK based on EPT_VIOLATION_GVA_TRANSLATED if
> and only if EPT_VIOLATION_GVA_IS_VALID is also set in exit qualification.
> Per the SDM, bit 8 (EPT_VIOLATION_GVA_TRANSLATED) is valid if and only if
> bit 7 (EPT_VIOLATION_GVA_IS_VALID) is set, and is '0' if bit 7 is '0'.
>
> Bit 7 (a.k.a. EPT_VIOLATION_GVA_IS_VALID)
>
> Set if the guest linear-address field is valid. The guest linear-address
> field is valid for all EPT violations except those resulting from an
> attempt to load the guest PDPTEs as part of the execution of the MOV CR
> instruction and those due to trace-address pre-translation
>
> Bit 8 (a.k.a. EPT_VIOLATION_GVA_TRANSLATED)
>
> If bit 7 is 1:
> • Set if the access causing the EPT violation is to a guest-physical
> address that is the translation of a linear address.
> • Clear if the access causing the EPT violation is to a paging-structure
> entry as part of a page walk or the update of an accessed or dirty bit.
> Reserved if bit 7 is 0 (cleared to 0).
>
> Failure to guard the logic on GVA_IS_VALID results in KVM marking the page
> fault as PFERR_GUEST_PAGE_MASK when there is no known GVA, which can put
> the vCPU into an infinite loop due to kvm_mmu_page_fault() getting false
> positive on its PFERR_NESTED_GUEST_PAGE logic (though only because that
> logic is also buggy/flawed).
>
> In practice, this is largely a non-issue because so GVA_IS_VALID is almost
> always set. However, when TDX comes along, GVA_IS_VALID will *never* be
> set, as the TDX Module deliberately clears bits 12:7 in exit qualification,
> e.g. so that the faulting virtual address and other metadata that aren't
> practically useful for the hypervisor aren't leaked to the untrusted host.
>
> When exit is due to EPT violation, bits 12-7 of the exit qualification
> are cleared to 0.
>
> Fixes: eebed2438923 ("kvm: nVMX: Add support for fast unprotection of nested guest page tables")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx/vmx.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f18c2d8c7476..52de013550e9 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5804,8 +5804,9 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
> error_code |= (exit_qualification & EPT_VIOLATION_RWX_MASK)
> ? PFERR_PRESENT_MASK : 0;
>
> - error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
> - PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
> + if (error_code & EPT_VIOLATION_GVA_IS_VALID)
> + error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) ?
> + PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
Reviewed-by: Yuan Yao <yuan.yao@xxxxxxxxx>
>
> /*
> * Check that the GPA doesn't exceed physical memory limits, as that is
> --
> 2.46.0.76.ge559c4bf1a-goog
>
>
