Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch incompletely mimics VMRUN hardware behavior for EFER.LMA, allowing a malicious guest to desynchronize KVM's MMU state from the physical CPU. While the patch forces EFER.LMA=0 when EFER.LME=0, it fails to set EFER.LMA=1 when EFER.LME=1 and CR0.PG=1.
--

commit 0061c7315461e9b92e5efb9b382e2eecda3b30bb
Author: Sean Christopherson <seanjc@google.com>

KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

This commit forces EFER.LMA=0 if EFER.LME=0 when preparing L2 state for
VMRUN, mimicking hardware behavior. It addresses issues in MMU code that
key off EFER.LMA when determining whether to emulate 4/5-level paging
versus PAE paging.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 49fb10ad1f9f6..23d29597d6bf2 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  
>  	kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
>  
> +	/* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> +	if (!(svm->nested.save.efer & EFER_LME))
> +		svm->nested.save.efer &= ~EFER_LMA;
> +
>  	svm_set_efer(vcpu, svm->nested.save.efer);
>  
>  	svm_set_cr0(vcpu, svm->nested.save.cr0);

[Severity: High]
This is a pre-existing issue, but does this logic only implement half of the
hardware VMRUN rule? 

While this correctly fixes the invariant for EFER.LME=0, could a malicious
guest desynchronize KVM's MMU state from the physical CPU when EFER.LME=1?

If an L1 guest creates a VMCB12 with EFER.LME=1, CR0.PG=1, but EFER.LMA=0,
and executes VMRUN, nested_vmcb02_prepare_save() skips the LMA update
because LME=1, passing LMA=0 to svm_set_efer().

Then, when we reach svm_set_cr0(), it fails to correct EFER.LMA because it
expects an edge transition (!is_paging(vcpu)), which is always false since
L1 must already be paging to execute VMRUN:

arch/x86/kvm/svm/svm.c:svm_set_cr0() {
    ...
    if (vcpu->arch.efer & EFER_LME) {
        if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
            vcpu->arch.efer |= EFER_LMA;
    ...
}

Would this allow the guest to trick KVM's instruction emulator into fetching
arbitrary instructions from a different physical page than what the guest
actually executed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902232028.2767071-1-seanjc@google.com?part=2