Re: [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: x86: OOB read and type confusion via static SVM feature check
--

commit ed7805af977b5a574d9ccec8367687b0fbc85fd3
Author: Kyle Zeng <kylebot@openai.com>

KVM: x86: Restrict saved GPA writes to hardware write faults

This commit restricts emulated writes through a saved GPA to cases where
the hardware page fault actually reported a write access. It includes an
exception for SEV guests to preserve existing behavior since their guest
page tables are encrypted.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 79468ddfe4736..de7fc8efeadce 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -33,6 +33,7 @@
>  #include "lapic.h"
>  #include "xen.h"
>  #include "smm.h"
> +#include "svm/svm.h"

[Severity: High]
Does including a vendor-specific internal header like "svm/svm.h" inside
generic architecture code violate KVM layering rules?

[ ... ]

> @@ -6454,7 +6456,18 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>  
>  		/* With shadow page tables, cr2 contains a GVA or nGPA. */
>  		if (vcpu->arch.mmu->root_role.direct) {
> -			ctxt->gpa_available = true;
> +			ctxt->gpa_access = ACC_READ_MASK;
> +			/*
> +			 * Always allow writes for SEV guests, as the guest's
> +			 * page tables are encrypted, i.e. KVM can't walk the
> +			 * guest's page tables and so must always use the GPA
> +			 * from the initial fault.  Restricting use of the GPA
> +			 * to the access type that faulted would prevent KVM
> +			 * from emulating RMW operations for SEV guests.
> +			 */
> +			if ((emulation_type & EMULTYPE_PF_WRITE) ||
> +			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))

[Severity: High]
Could this lead to an out-of-bounds read and type confusion?

The cpu_feature_enabled(X86_FEATURE_SVM) check relies on a static host CPU
capability rather than checking the active KVM hypervisor module. In a nested
virtualization scenario (like QEMU with -cpu max), the host can expose both
VMX and SVM capabilities, but the kvm-intel module might be the one managing
the VM.

If a guest triggers an emulation fault on a kvm_vmx object, this path could
call is_sev_guest(vcpu). Since is_sev_guest() expects a kvm_svm vCPU, it
will blindly cast the kvm_vmx object to kvm_svm and read memory past the end
of the kvm_vmx allocation. 

If that out-of-bounds memory happens to be non-zero, it would incorrectly
grant ACC_WRITE_MASK authorization for emulated writes, leading to a
potential security bypass or host kernel panic.

> +				ctxt->gpa_access |= ACC_WRITE_MASK;
>  			ctxt->gpa_val = cr2_or_gpa;
>  		}
>  	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915222625.99965-1-kylebot@openai.com?part=1