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

[Kyle Zeng <kylebot@openai.com>]

On Tue, Sep 15, 2026 at 10:41:03PM +0000, sashiko-bot@kernel.org wrote:
> 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

On this baseline, x86_virt_init() explicitly refuses to enable
virtualization when both VMX and SVM initialize successfully. VM creation
then fails, so exposing both CPUID bits doesn't appear sufficient to
reach the path described here.

Also, is_sev_guest() casts vcpu->kvm, not the vCPU. In the configuration
I built, the access would fall within zeroed padding in struct kvm_vmx,
not beyond the allocation. That's not something to rely on for
correctness, but it doesn't support the claimed OOB either.

Is there a configuration that gets past the initialization checks and
demonstrates an incorrect access? I'd like to establish that before
treating this as a functional issue.

Thanks,
Kyle