|
 |
983fad |
From 08a95c3541cbe2b3a1c671fa683bd6214ad996f0 Mon Sep 17 00:00:00 2001
|
|
|
983fad |
From: Laszlo Ersek <lersek@redhat.com>
|
|
|
983fad |
Date: Thu, 27 Aug 2020 00:21:29 +0200
|
|
|
983fad |
Subject: [PATCH 3/5] OvmfPkg/CpuHotplugSmm: fix CPU hotplug race just after
|
|
|
983fad |
SMI broadcast
|
|
|
983fad |
MIME-Version: 1.0
|
|
|
983fad |
Content-Type: text/plain; charset=UTF-8
|
|
|
983fad |
Content-Transfer-Encoding: 8bit
|
|
|
983fad |
|
|
|
983fad |
RH-Author: Laszlo Ersek (lersek)
|
|
|
983fad |
RH-MergeRequest: 1: [RHEL-8.4.0] complete the "VCPU hotplug with SMI" OVMF feature
|
|
|
983fad |
RH-Commit: [3/3] 40521ea89725b8b0ff8ca3f0a610ff45431e610e (lersek/edk2)
|
|
|
983fad |
RH-Bugzilla: 1849177
|
|
|
983fad |
|
|
|
983fad |
The "virsh setvcpus" (plural) command may hot-plug several VCPUs in quick
|
|
|
983fad |
succession -- it means a series of "device_add" QEMU monitor commands,
|
|
|
983fad |
back-to-back.
|
|
|
983fad |
|
|
|
983fad |
If a "device_add" occurs *just after* ACPI raises the broadcast SMI, then:
|
|
|
983fad |
|
|
|
983fad |
- the CPU_FOREACH() loop in QEMU's ich9_apm_ctrl_changed() cannot make the
|
|
|
983fad |
SMI pending for the new CPU -- at that time, the new CPU doesn't even
|
|
|
983fad |
exist yet,
|
|
|
983fad |
|
|
|
983fad |
- OVMF will find the new CPU however (in the CPU hotplug register block),
|
|
|
983fad |
in QemuCpuhpCollectApicIds().
|
|
|
983fad |
|
|
|
983fad |
As a result, when the firmware sends an INIT-SIPI-SIPI to the new CPU in
|
|
|
983fad |
SmbaseRelocate(), expecting it to boot into SMM (due to the pending SMI),
|
|
|
983fad |
the new CPU instead boots straight into the post-RSM (normal mode) "pen",
|
|
|
983fad |
skipping its initial SMI handler.
|
|
|
983fad |
|
|
|
983fad |
The CPU halts nicely in the pen, but its SMBASE is never relocated, and
|
|
|
983fad |
the SMRAM message exchange with the BSP falls apart -- the BSP gets stuck
|
|
|
983fad |
in the following loop:
|
|
|
983fad |
|
|
|
983fad |
//
|
|
|
983fad |
// Wait until the hot-added CPU is just about to execute RSM.
|
|
|
983fad |
//
|
|
|
983fad |
while (Context->AboutToLeaveSmm == 0) {
|
|
|
983fad |
CpuPause ();
|
|
|
983fad |
}
|
|
|
983fad |
|
|
|
983fad |
because the new CPU's initial SMI handler never sets the flag to nonzero.
|
|
|
983fad |
|
|
|
983fad |
Fix this by sending a directed SMI to the new CPU just before sending it
|
|
|
983fad |
the INIT-SIPI-SIPI. The various scenarios are documented in the code --
|
|
|
983fad |
the cases affected by the patch are documented under point (2).
|
|
|
983fad |
|
|
|
983fad |
Note that this is not considered a security patch, as for a malicious
|
|
|
983fad |
guest OS, the issue is not exploitable -- the symptom is a hang on the
|
|
|
983fad |
BSP, in the above-noted loop in SmbaseRelocate(). Instead, the patch fixes
|
|
|
983fad |
behavior for a benign guest OS.
|
|
|
983fad |
|
|
|
983fad |
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
|
|
|
983fad |
Cc: Igor Mammedov <imammedo@redhat.com>
|
|
|
983fad |
Cc: Jordan Justen <jordan.l.justen@intel.com>
|
|
|
983fad |
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
983fad |
Fixes: 51a6fb41181529e4b50ea13377425bda6bb69ba6
|
|
|
983fad |
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2929
|
|
|
983fad |
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
983fad |
Message-Id: <20200826222129.25798-3-lersek@redhat.com>
|
|
|
983fad |
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
|
|
|
983fad |
(cherry picked from commit cbccf995920a28071f5403b847f29ebf8b732fa9)
|
|
|
983fad |
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
983fad |
---
|
|
|
983fad |
OvmfPkg/CpuHotplugSmm/Smbase.c | 35 ++++++++++++++++++++++++++++------
|
|
|
983fad |
1 file changed, 29 insertions(+), 6 deletions(-)
|
|
|
983fad |
|
|
|
983fad |
diff --git a/OvmfPkg/CpuHotplugSmm/Smbase.c b/OvmfPkg/CpuHotplugSmm/Smbase.c
|
|
|
983fad |
index 170571221d..d8f45c4313 100644
|
|
|
983fad |
--- a/OvmfPkg/CpuHotplugSmm/Smbase.c
|
|
|
983fad |
+++ b/OvmfPkg/CpuHotplugSmm/Smbase.c
|
|
|
983fad |
@@ -220,14 +220,37 @@ SmbaseRelocate (
|
|
|
983fad |
//
|
|
|
983fad |
// Boot the hot-added CPU.
|
|
|
983fad |
//
|
|
|
983fad |
- // If the OS is benign, and so the hot-added CPU is still in RESET state,
|
|
|
983fad |
- // then the broadcast SMI is still pending for it; it will now launch
|
|
|
983fad |
- // directly into SMM.
|
|
|
983fad |
+ // There are 2*2 cases to consider:
|
|
|
983fad |
//
|
|
|
983fad |
- // If the OS is malicious, the hot-added CPU has been booted already, and so
|
|
|
983fad |
- // it is already spinning on the APIC ID gate. In that case, the
|
|
|
983fad |
- // INIT-SIPI-SIPI below will be ignored.
|
|
|
983fad |
+ // (1) The CPU was hot-added before the SMI was broadcast.
|
|
|
983fad |
//
|
|
|
983fad |
+ // (1.1) The OS is benign.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // The hot-added CPU is in RESET state, with the broadcast SMI pending
|
|
|
983fad |
+ // for it. The directed SMI below will be ignored (it's idempotent),
|
|
|
983fad |
+ // and the INIT-SIPI-SIPI will launch the CPU directly into SMM.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // (1.2) The OS is malicious.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // The hot-added CPU has been booted, by the OS. Thus, the hot-added
|
|
|
983fad |
+ // CPU is spinning on the APIC ID gate. In that case, both the SMI and
|
|
|
983fad |
+ // the INIT-SIPI-SIPI below will be ignored.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // (2) The CPU was hot-added after the SMI was broadcast.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // (2.1) The OS is benign.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // The hot-added CPU is in RESET state, with no SMI pending for it. The
|
|
|
983fad |
+ // directed SMI will latch the SMI for the CPU. Then the INIT-SIPI-SIPI
|
|
|
983fad |
+ // will launch the CPU into SMM.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // (2.2) The OS is malicious.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ // The hot-added CPU is executing OS code. The directed SMI will pull
|
|
|
983fad |
+ // the hot-added CPU into SMM, where it will start spinning on the APIC
|
|
|
983fad |
+ // ID gate. The INIT-SIPI-SIPI will be ignored.
|
|
|
983fad |
+ //
|
|
|
983fad |
+ SendSmiIpi (ApicId);
|
|
|
983fad |
SendInitSipiSipi (ApicId, PenAddress);
|
|
|
983fad |
|
|
|
983fad |
//
|
|
|
983fad |
--
|
|
|
983fad |
2.27.0
|
|
|
983fad |
|