[PATCH] KVM: Fix LDR inconsistency warning caused by APIC_ID format error

[Haoyu Wu]

Syzkaller detected a warning in the kvm_recalculate_logical_map()
function. This function employs VCPU_ID as the current x2APIC_ID
following the apic_x2apic_mode() check. However, the LDR value,
as computed using the current x2APIC_ID,  fails to align with the LDR
value that is actually set.

Syzkaller scenario:
1) Set up VCPU's
2) Set the APIC_BASE to 0xd00
3) Set the APIC status for a specific state

The issue arises within kvm_apic_state_fixup, a function responsible
for adjusting and correcting the APIC state. Initially, it verifies
whether the current vcpu operates in x2APIC mode by examining the
vcpu's mode. Subsequently, the function evaluates
vcpu->kvm->arch.x2apic_format to ascertain if the preceding kvm version
supports x2APIC mode. In cases where kvm is compatible with x2APIC mode,
the function compares APIC_ID and VCPU_ID for equality. If they are not
equal, it processes APIC_ID according to the set value. The error
manifests when vcpu->kvm->arch.x2apic_format is false; under these
circumstances, kvm_apic_state_fixup converts APIC_ID to the xAPIC format
and invokes kvm_apic_calc_x2apic_ldr to compute the LDR. This leads to by
passing consistency checks between VCPU_ID and APIC_ID and results in
calling incorrect functions for LDR calculation.

Obviously, the crux of the issue hinges on the transition of the APIC
state and the associated operations for transitioning APIC_ID. In the
current kernel design, APIC_ID defaults to VCPU_ID in x2APIC mode, a
specification not required in xAPIC mode. kvm_apic_state_fixup initiates
by assessing the current status of both VCPU and KVM to identify their
respective APIC modes. However, subsequent evaluations focus solely on
the APIC mode of VCPU. To address this, a feasible minor modification
involves advancing the comparison between APIC_ID and VCPU_ID,
positioning it prior to the evaluation of vcpu→kvm→arch.x2apic_format.

Signed-off-by: Haoyu Wu <haoyuwu254@xxxxxxxxx>
---
 arch/x86/kvm/lapic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 3242f3da2..16c97d57d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2933,16 +2933,16 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
 		u32 *ldr = (u32 *)(s->regs + APIC_LDR);
 		u64 icr;
 
-		if (vcpu->kvm->arch.x2apic_format) {
-			if (*id != vcpu->vcpu_id)
-				return -EINVAL;
-		} else {
+		if (*id != vcpu->vcpu_id)
+			return -EINVAL;
+		if (!vcpu->kvm->arch.x2apic_format) {
 			if (set)
 				*id >>= 24;
 			else
 				*id <<= 24;
 		}
 
+
 		/*
 		 * In x2APIC mode, the LDR is fixed and based on the id.  And
 		 * ICR is internally a single 64-bit register, but needs to be
-- 
2.34.1