linux-kernel - Re: [PATCH 02/22] KVM: VMX: Set PFERR_GUEST_{FINAL,PAGE}

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240814111134.susqam5ecwdb26ky@yy-desk-7060>
Date: Wed, 14 Aug 2024 19:11:34 +0800
From: Yuan Yao <yuan.yao@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Peter Gonda <pgonda@...gle.com>,
	Michael Roth <michael.roth@....com>,
	Vishal Annapurve <vannapurve@...gle.com>,
	Ackerly Tng <ackerleytng@...gle.com>
Subject: Re: [PATCH 02/22] KVM: VMX: Set PFERR_GUEST_{FINAL,PAGE}_MASK if and
 only if the GVA is valid

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@...r.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>  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@...el.com>

>
>  	/*
>  	 * Check that the GPA doesn't exceed physical memory limits, as that is
> --
> 2.46.0.76.ge559c4bf1a-goog
>
>