[PATCH v3 0/2] KVM: x86: MCE fixes

[PATCH v3 0/2] KVM: x86: MCE fixes

[Carlos López]

These two patches are somewhat unrelated, but patch 2 came out of
Sean's suggestions from reviewing the first patch (see [1]). Patch 1 is
unchanged.

v3:
  * Address Sashiko's review. While incorrect in pointing out MCIP's
    role, it was partially correct in the sense that the MCE logging
    rules were still not fully compliant to the SDM.

v2:
  * Add patch 2.

[1] https://lore.kernel.org/all/20260516163412.601908-1-clopez@suse.de/

Carlos López (2):
  KVM: x86: Fix array_index_nospec() protection in
    kvm_vcpu_ioctl_x86_set_mce()
  KVM: x86: Fix MCE logging rules for KVM_X86_SET_MCE

 arch/x86/kvm/x86.c | 65 +++++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 24 deletions(-)


base-commit: de3a35be92d2391ece4bf3143ef2887192625fd0
-- 
2.51.0

[PATCH v3 1/2] KVM: x86: Fix array_index_nospec() protection in kvm_vcpu_ioctl_x86_set_mce()

[Carlos López]

Commit aebc3ca19063 ("KVM: x86: Enable CMCI capability by default and
handle injected UCNA errors") introduced kvm_vcpu_x86_set_ucna(), which
accesses @vcpu->arch.mci_ctl2_banks[] using @mce->bank as the index. The
@mce struct is user-controlled, provided via the KVM_X86_SET_MCE ioctl.

The caller of this function, kvm_vcpu_ioctl_x86_set_mce(), bounds-checks
@mce->bank and applies array_index_nospec() to advance the @banks
pointer, but @mce->bank itself is passed through unclamped. On a
speculative path that bypasses the bounds check, the raw @mce->bank
value can index mci_ctl2_banks[] out-of-bounds.

In practice this is a very weak gadget, and would at most allow leaking
a single bit in a 64-bit integer, but prevent potential future issues by
clamping @mce->bank in place with array_index_nospec(), before passing
the struct to kvm_vcpu_x86_set_ucna().

Fixes: aebc3ca19063 ("KVM: x86: Enable CMCI capability by default and handle injected UCNA errors")
Signed-off-by: Carlos López <clopez@suse.de>
---
 arch/x86/kvm/x86.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cf122b8c3210..77a780177c4e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5472,7 +5472,8 @@ static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
 	if (mce->bank >= bank_num || !(mce->status & MCI_STATUS_VAL))
 		return -EINVAL;
 
-	banks += array_index_nospec(4 * mce->bank, 4 * bank_num);
+	mce->bank = array_index_nospec(mce->bank, bank_num);
+	banks += 4 * mce->bank;
 
 	if (is_ucna(mce))
 		return kvm_vcpu_x86_set_ucna(vcpu, mce, banks);
-- 
2.51.0

[PATCH v3 2/2] KVM: x86: Fix MCE logging rules for KVM_X86_SET_MCE

[Carlos López]

When userspace issues KVM_X86_SET_MCE, kvm_vcpu_ioctl_x86_set_mce()
decides whether to log an uncorrectable MCE by looking at the
corresponding IA32_MCi_CTL MSR. This is not the behavior specified in
the Intel SDM (17.3.2.1 IA32_MCi_CTL MSRs):

  Setting an EEj flag enables signaling #MC of the associated error and
  clearing it disables signaling of the error. Error logging happens
  regardless of the setting of these bits. The processor drops writes to
  bits that are not implemented.

Perform the logging before checking MCi_CTL, unless there is already
a valid UC error logged for the bank, in which case the SDM (17.3.2.2
"IA32_MCi_STATUS MSRS") specifies that error information should not
be overwritten.

To avoid even more complex control flow, hoist the logging logic into a
separate function, which then enables the removal of the non-UC branch
in kvm_vcpu_ioctl_x86_set_mce(), which only existed to perform logging.

Note that the SDM is ambiguous regarding the effects of IA32_MCG_CTL on
logging, so preserve the existing logic (i.e. do not log the error if
MCG_CTL is disabled).

Fixes: 890ca9aefa78 ("KVM: Add MCE support")
Signed-off-by: Carlos López <clopez@suse.de>
---
 arch/x86/kvm/x86.c | 62 +++++++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 77a780177c4e..af3662aa3ce3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5462,6 +5462,28 @@ static int kvm_vcpu_x86_set_ucna(struct kvm_vcpu *vcpu, struct kvm_x86_mce *mce,
 	return 0;
 }
 
+/*
+ * Record @mce into @banks per the SDM logging rules:
+ *   - Per 17.3.2.1, logging happens regardless of IA32_MCi_CTL.
+ *   - Per 17.3.2.2.1 and 17.3.2.2, a UC error does not overwrite a
+ *     previous valid UC log in the same bank; only OVER is set.
+ *     Any other prior state (invalid, or valid non-UC) is replaced,
+ *     with OVER set if a prior valid log was overwritten.
+ */
+static void kvm_log_mce(u64 *banks, struct kvm_x86_mce *mce)
+{
+	bool overflow = banks[1] & MCI_STATUS_VAL;
+
+	if (!overflow || !(banks[1] & MCI_STATUS_UC)) {
+		banks[2] = mce->addr;
+		banks[3] = mce->misc;
+		banks[1] = mce->status;
+	}
+
+	if (overflow)
+		banks[1] |= MCI_STATUS_OVER;
+}
+
 static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
 				      struct kvm_x86_mce *mce)
 {
@@ -5485,34 +5507,28 @@ static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
 	if ((mce->status & MCI_STATUS_UC) && (mcg_cap & MCG_CTL_P) &&
 	    vcpu->arch.mcg_ctl != ~(u64)0)
 		return 0;
+
+	kvm_log_mce(banks, mce);
+
+	if (!(mce->status & MCI_STATUS_UC))
+		return 0;
+
 	/*
 	 * if IA32_MCi_CTL is not all 1s, the uncorrected error
 	 * reporting is disabled for the bank
 	 */
-	if ((mce->status & MCI_STATUS_UC) && banks[0] != ~(u64)0)
+	if (banks[0] != ~(u64)0)
 		return 0;
-	if (mce->status & MCI_STATUS_UC) {
-		if ((vcpu->arch.mcg_status & MCG_STATUS_MCIP) ||
-		    !kvm_is_cr4_bit_set(vcpu, X86_CR4_MCE)) {
-			kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
-			return 0;
-		}
-		if (banks[1] & MCI_STATUS_VAL)
-			mce->status |= MCI_STATUS_OVER;
-		banks[2] = mce->addr;
-		banks[3] = mce->misc;
-		vcpu->arch.mcg_status = mce->mcg_status;
-		banks[1] = mce->status;
-		kvm_queue_exception(vcpu, MC_VECTOR);
-	} else if (!(banks[1] & MCI_STATUS_VAL)
-		   || !(banks[1] & MCI_STATUS_UC)) {
-		if (banks[1] & MCI_STATUS_VAL)
-			mce->status |= MCI_STATUS_OVER;
-		banks[2] = mce->addr;
-		banks[3] = mce->misc;
-		banks[1] = mce->status;
-	} else
-		banks[1] |= MCI_STATUS_OVER;
+
+	if ((vcpu->arch.mcg_status & MCG_STATUS_MCIP) ||
+	    !kvm_is_cr4_bit_set(vcpu, X86_CR4_MCE)) {
+		kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
+		return 0;
+	}
+
+	vcpu->arch.mcg_status = mce->mcg_status;
+	kvm_queue_exception(vcpu, MC_VECTOR);
+
 	return 0;
 }
 
-- 
2.51.0

Re: [PATCH v3 0/2] KVM: x86: MCE fixes

[Sean Christopherson]

On Tue, 09 Jun 2026 15:18:54 +0200, Carlos López wrote:
> These two patches are somewhat unrelated, but patch 2 came out of
> Sean's suggestions from reviewing the first patch (see [1]). Patch 1 is
> unchanged.
> 
> v3:
>   * Address Sashiko's review. While incorrect in pointing out MCIP's
>     role, it was partially correct in the sense that the MCE logging
>     rules were still not fully compliant to the SDM.
> 
> [...]

Applied patch 1 to kvm-x86 misc so it doesn't get waylaid any longer.  I'll
respond to patch 2 at some point (hopefully today, but it might not be until
next week).

[1/2] KVM: x86: Fix array_index_nospec() protection in kvm_vcpu_ioctl_x86_set_mce()
      https://github.com/kvm-x86/linux/commit/249f9be58c7f

--
https://github.com/kvm-x86/linux/tree/next