linux-kernel - [PATCH] KVM: x86: preserve interrupt shadow across SMM entries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220607151647.307157-1-mlevitsk@redhat.com>
Date:   Tue,  7 Jun 2022 18:16:47 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     kvm@...r.kernel.org
Cc:     x86@...nel.org, Borislav Petkov <bp@...en8.de>,
        Paolo Bonzini <pbonzini@...hat.com>,
        linux-kernel@...r.kernel.org,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Jim Mattson <jmattson@...gle.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>
Subject: [PATCH] KVM: x86: preserve interrupt shadow across SMM entries

If the #SMI happens while the vCPU is in the interrupt shadow,
(after STI or MOV SS),
we must both clear it to avoid VM entry failure on VMX,
due to consistency check vs EFLAGS.IF which is cleared on SMM entries,
and restore it on RSM so that #SMI is transparent to the non SMM code.

To support migration, reuse upper 4 bits of
'kvm_vcpu_events.interrupt.shadow' to store the smm interrupt shadow.

This was lightly tested with a linux guest and smm load script,
and a unit test will be soon developed to test this better.

For discussion: there are other ways to fix this issue:

1. The SMM shadow can be stored in SMRAM at some unused
offset, this will allow to avoid changes to kvm_vcpu_ioctl_x86_set_vcpu_events

2. #SMI can instead be blocked while the interrupt shadow is active,
which might even be what the real CPU does, however since neither VMX
nor SVM support SMM window handling, this will involve single stepping
the guest like it is currently done on SVM for the NMI window in some cases.

What do you think?

Also this might need a new KVM cap, I am not sure about it.

Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
---
 arch/x86/include/asm/kvm_host.h |  3 +++
 arch/x86/kvm/x86.c              | 28 +++++++++++++++++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 6cf5d77d78969..4ee14dc79646f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -939,6 +939,9 @@ struct kvm_vcpu_arch {
 	 */
 	bool pdptrs_from_userspace;
 
+	/* saved interrupt shadow on smm entry */
+	u8 smm_interrupt_shadow;
+
 #if IS_ENABLED(CONFIG_HYPERV)
 	hpa_t hv_root_tdp;
 #endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2db6f0373fa3f..28d736b74eec6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4915,6 +4915,8 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
 	events->interrupt.soft = 0;
 	events->interrupt.shadow = static_call(kvm_x86_get_interrupt_shadow)(vcpu);
 
+	events->interrupt.shadow |= vcpu->arch.smm_interrupt_shadow << 4;
+
 	events->nmi.injected = vcpu->arch.nmi_injected;
 	events->nmi.pending = vcpu->arch.nmi_pending != 0;
 	events->nmi.masked = static_call(kvm_x86_get_nmi_mask)(vcpu);
@@ -4988,9 +4990,6 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 	vcpu->arch.interrupt.injected = events->interrupt.injected;
 	vcpu->arch.interrupt.nr = events->interrupt.nr;
 	vcpu->arch.interrupt.soft = events->interrupt.soft;
-	if (events->flags & KVM_VCPUEVENT_VALID_SHADOW)
-		static_call(kvm_x86_set_interrupt_shadow)(vcpu,
-						events->interrupt.shadow);
 
 	vcpu->arch.nmi_injected = events->nmi.injected;
 	if (events->flags & KVM_VCPUEVENT_VALID_NMI_PENDING)
@@ -5024,6 +5023,14 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 		}
 	}
 
+	if (events->flags & KVM_VCPUEVENT_VALID_SHADOW) {
+		static_call(kvm_x86_set_interrupt_shadow)(vcpu, events->interrupt.shadow & 0xF);
+		if (events->flags & KVM_VCPUEVENT_VALID_SMM)
+			vcpu->arch.smm_interrupt_shadow = events->interrupt.shadow >> 4;
+		else
+			vcpu->arch.smm_interrupt_shadow = 0;
+	}
+
 	if (events->flags & KVM_VCPUEVENT_VALID_TRIPLE_FAULT) {
 		if (!vcpu->kvm->arch.triple_fault_event)
 			return -EINVAL;
@@ -8282,6 +8289,15 @@ static void kvm_smm_changed(struct kvm_vcpu *vcpu, bool entering_smm)
 
 	if (entering_smm) {
 		vcpu->arch.hflags |= HF_SMM_MASK;
+
+		/* Stash aside current value of the interrupt shadow
+		 * and reset it on the entry to the SMM
+		 */
+		vcpu->arch.smm_interrupt_shadow =
+				static_call(kvm_x86_get_interrupt_shadow)(vcpu);
+
+		static_call(kvm_x86_set_interrupt_shadow)(vcpu, 0);
+
 	} else {
 		vcpu->arch.hflags &= ~(HF_SMM_MASK | HF_SMM_INSIDE_NMI_MASK);
 
@@ -8294,6 +8310,12 @@ static void kvm_smm_changed(struct kvm_vcpu *vcpu, bool entering_smm)
 		 * guest memory
 		 */
 		vcpu->arch.pdptrs_from_userspace = false;
+
+		/* Restore interrupt shadow to its pre-SMM value */
+		static_call(kvm_x86_set_interrupt_shadow)(vcpu,
+					vcpu->arch.smm_interrupt_shadow);
+
+		vcpu->arch.smm_interrupt_shadow = 0;
 	}
 
 	kvm_mmu_reset_context(vcpu);
-- 
2.31.1