[PATCH 0/2] KVM: x86: Fix RM exception injection bugs

Sean Christopherson posted 2 patches 3 years ago
arch/x86/kvm/vmx/nested.c |  7 ++++++-
arch/x86/kvm/x86.c        | 11 +++++++++--
2 files changed, 15 insertions(+), 3 deletions(-)
[PATCH 0/2] KVM: x86: Fix RM exception injection bugs
Posted by Sean Christopherson 3 years ago
Fix two bugs introduced by a semi-recent fix for AMD's Page Real Mode.

Patch 1 was tested against the syzkaller testcase that exposed the bug[*].

Patch 2 was testing by enabling the VMware backdoor in L1 to turn on #GP
interception, hacking L0 KVM to passthrough MSR_IA32_VMX_BASIC to L1
and then writing the MSR from L2 while in Real Mode, thus forcing L0 KVM
to emulate the WRMSR from L2 and synthesiz a #GP VM-Exit into L1.
Confirmed that bad behavior in L1 with an assertion:

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index bcac3efcde41..ef3bb5ab9654 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5178,6 +5178,9 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
        vect_info = vmx->idt_vectoring_info;
        intr_info = vmx_get_intr_info(vcpu);
 
+       WARN_ONCE((intr_info & INTR_INFO_DELIVER_CODE_MASK) && !is_protmode(vcpu),
+                 "Exception VM-Exit shouldn't report error code when CPU is in Real Mode");
+
        /*
         * Machine checks are handled by handle_exception_irqoff(), or by
         * vmx_vcpu_run() if a #MC occurs on VM-Entry.  NMIs are handled by

[*] https://lkml.kernel.org/r/ZBNrWZQhMX8AHzWM%40google.com

Sean Christopherson (2):
  KVM: x86: Clear "has_error_code", not "error_code", for RM exception
    injection
  KVM: nVMX: Do not report error code when synthesizing VM-Exit from
    Real Mode

 arch/x86/kvm/vmx/nested.c |  7 ++++++-
 arch/x86/kvm/x86.c        | 11 +++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)


base-commit: 45dd9bc75d9adc9483f0c7d662ba6e73ed698a0b
-- 
2.40.0.rc2.332.ga46443480c-goog
Re: [PATCH 0/2] KVM: x86: Fix RM exception injection bugs
Posted by Paolo Bonzini 3 years ago
Queued, thanks.

Paolo
[PATCH 1/2] KVM: x86: Clear "has_error_code", not "error_code", for RM exception injection
Posted by Sean Christopherson 3 years ago
When injecting an exception into a vCPU in Real Mode, suppress the error
code by clearing the flag that tracks whether the error code is valid, not
by clearing the error code itself.  The "typo" was introduced by recent
fix for SVM's funky Paged Real Mode.

Opportunistically hoist the logic above the tracepoint so that the trace
is coherent with respect to what is actually injected (this was also the
behavior prior to the buggy commit).

Fixes: b97f07458373 ("KVM: x86: determine if an exception has an error code only when injecting it.")
Cc: stable@vger.kernel.org
Cc: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f706621c35b8..e74aaf57eab5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9884,13 +9884,20 @@ int kvm_check_nested_events(struct kvm_vcpu *vcpu)
 
 static void kvm_inject_exception(struct kvm_vcpu *vcpu)
 {
+	/*
+	 * Suppress the error code if the vCPU is in Real Mode, as Real Mode
+	 * exceptions don't report error codes.  The presence of an error code
+	 * is carried with the exception and only stripped when the exception
+	 * is injected as intercepted #PF VM-Exits for AMD's Paged Real Mode do
+	 * report an error code despite the CPU being in Real Mode.
+	 */
+	vcpu->arch.exception.has_error_code &= is_protmode(vcpu);
+
 	trace_kvm_inj_exception(vcpu->arch.exception.vector,
 				vcpu->arch.exception.has_error_code,
 				vcpu->arch.exception.error_code,
 				vcpu->arch.exception.injected);
 
-	if (vcpu->arch.exception.error_code && !is_protmode(vcpu))
-		vcpu->arch.exception.error_code = false;
 	static_call(kvm_x86_inject_exception)(vcpu);
 }
 
-- 
2.40.0.rc2.332.ga46443480c-goog
[PATCH 2/2] KVM: nVMX: Do not report error code when synthesizing VM-Exit from Real Mode
Posted by Sean Christopherson 3 years ago
Don't report an error code to L1 when synthesizing a nested VM-Exit and
L2 is in Real Mode.  Per Intel's SDM, regarding the error code valid bit:

  This bit is always 0 if the VM exit occurred while the logical processor
  was in real-address mode (CR0.PE=0).

The bug was introduced by a recent fix for AMD's Paged Real Mode, which
moved the error code suppression from the common "queue exception" path
to the "inject exception" path, but missed VMX's "synthesize VM-Exit"
path.

Fixes: b97f07458373 ("KVM: x86: determine if an exception has an error code only when injecting it.")
Cc: stable@vger.kernel.org
Cc: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 7c4f5ca405c7..14be92b3f34c 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3868,7 +3868,12 @@ static void nested_vmx_inject_exception_vmexit(struct kvm_vcpu *vcpu)
 		exit_qual = 0;
 	}
 
-	if (ex->has_error_code) {
+	/*
+	 * Unlike AMD's Paged Real Mode, which reports an error code on #PF
+	 * VM-Exits even if the CPU is in Real Mode, Intel VMX never sets the
+	 * "has error code" flags on VM-Exit if the CPU is in Real Mode.
+	 */
+	if (ex->has_error_code && is_protmode(vcpu)) {
 		/*
 		 * Intel CPUs do not generate error codes with bits 31:16 set,
 		 * and more importantly VMX disallows setting bits 31:16 in the
-- 
2.40.0.rc2.332.ga46443480c-goog