linux-kernel - Re: [PATCH] KVM: x86: Don't read guest CR3 in async pf flow when guest state is protected

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] [day] [month] [year] [list]

Message-ID: <aTrvtszjqUFu4Svk@google.com>
Date: Thu, 11 Dec 2025 08:22:14 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Xiaoyao Li <xiaoyao.li@...el.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Rick Edgecombe <rick.p.edgecombe@...el.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Farrah Chen <farrah.chen@...el.com>
Subject: Re: [PATCH] KVM: x86: Don't read guest CR3 in async pf flow when
 guest state is protected

On Thu, Dec 11, 2025, Xiaoyao Li wrote:
> ---
> For AMD SEV-ES and SNP cases, the guest state is also protected. But
> unlike TDX, reading guest CR3 doesn't cause issue since CR3 is always
> marked available for svm vCPUs. It always gets the initial value 0,
> set by kvm_vcpu_reset(). Whether to update vcpu->arch.regs_avail to
> reflect the correct value for SEV-ES and SNP is another topic.
> ---
>  arch/x86/kvm/mmu/mmu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 667d66cf76d5..03be521df6b9 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4521,7 +4521,8 @@ static bool kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu,
>  	arch.gfn = fault->gfn;
>  	arch.error_code = fault->error_code;
>  	arch.direct_map = vcpu->arch.mmu->root_role.direct;
> -	arch.cr3 = kvm_mmu_get_guest_pgd(vcpu, vcpu->arch.mmu);
> +	arch.cr3 = vcpu->arch.guest_state_protected ? 0 :
> +		   kvm_mmu_get_guest_pgd(vcpu, vcpu->arch.mmu);
>  
>  	return kvm_setup_async_pf(vcpu, fault->addr,
>  				  kvm_vcpu_gfn_to_hva(vcpu, fault->gfn), &arch);
> @@ -4543,7 +4544,8 @@ void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu, struct kvm_async_pf *work)
>  		return;
>  
>  	if (!vcpu->arch.mmu->root_role.direct &&
> -	      work->arch.cr3 != kvm_mmu_get_guest_pgd(vcpu, vcpu->arch.mmu))
> +	    (vcpu->arch.guest_state_protected ||
> +	     work->arch.cr3 != kvm_mmu_get_guest_pgd(vcpu, vcpu->arch.mmu)))
>  		return;

Protected guests aren't compatible with shadow paging, so I'd rather key off the
direct MMU role.  '0' is also a legal address; INVALID_GPA would be better.

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 02c450686b4a..446bf2716d08 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4521,7 +4521,10 @@ static bool kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu,
        arch.gfn = fault->gfn;
        arch.error_code = fault->error_code;
        arch.direct_map = vcpu->arch.mmu->root_role.direct;
-       arch.cr3 = kvm_mmu_get_guest_pgd(vcpu, vcpu->arch.mmu);
+       if (arch.direct_map)
+               arch.cr3 = INVALID_GPA;
+       else
+               arch.cr3 = kvm_mmu_get_guest_pgd(vcpu, vcpu->arch.mmu);
 
        return kvm_setup_async_pf(vcpu, fault->addr,
                                  kvm_vcpu_gfn_to_hva(vcpu, fault->gfn), &arch);