diff options
author | Star Zeng <star.zeng@intel.com> | 2017-12-19 09:59:50 +0800 |
---|---|---|
committer | Jian J Wang <jian.j.wang@intel.com> | 2019-05-14 15:54:57 +0800 |
commit | cf574f0a1838665498e43e9c029d8c1211cbbfa3 (patch) | |
tree | f223c6a3c6a43176905214b5b0df1a8a4d1087e0 /UefiCpuPkg/PiSmmCpuDxeSmm | |
parent | 83f997e58dcab4238dc537f38730d98bf5dad0df (diff) | |
download | edk2-cf574f0a1838665498e43e9c029d8c1211cbbfa3.tar.gz edk2-cf574f0a1838665498e43e9c029d8c1211cbbfa3.tar.bz2 edk2-cf574f0a1838665498e43e9c029d8c1211cbbfa3.zip |
UefiCpuPkg PiSmmCpuDxeSmm: Only support IN/OUT IO save state read (CVE-2018-12182)
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1136
CVE: CVE-2018-12182
Customer met system hang-up during serial port loopback test in OS.
It is a corner case happened with one CPU core doing "out dx,al" and
another CPU core(s) doing "rep outs dx,byte ptr [rsi]".
Detailed code flow is as below.
1. Serial port loopback test in OS.
One CPU core: "out dx,al" -> Writing B2h, SMI will happen.
Another CPU core(s): "rep outs dx,byte ptr [rsi]".
2. SMI happens to enter SMM.
"out dx" (SMM_IO_TYPE_OUT_DX) is saved as I/O instruction type in
SMRAM save state for CPU doing "out dx,al".
"rep outs dx" (SMM_IO_TYPE_REP_OUTS) is saved as I/O instruction
type and rsi is save as I/O Memory Address in SMRAM save state for
CPU doing "rep outs dx, byte ptr [rsi]".
NOTE: I/O Memory Address (rsi) is a virtual address mapped by
OS/Virtual Machine.
3. Some SMM code calls EFI_SMM_CPU_PROTOCOL.ReadSaveState() with
EFI_SMM_SAVE_STATE_REGISTER_IO and parse data returned.
For example:
https://github.com/tianocore/edk2/blob/master/QuarkSocPkg/
QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmSw.c#L76
4. SmmReadSaveState() is executed to read save state for
EFI_SMM_SAVE_STATE_REGISTER_IO.
- The SmmReadSaveState() function in
"UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c" calls the
SmmCpuFeaturesReadSaveStateRegister() function, from the platform's
SmmCpuFeaturesLib instance.
- If that platform-specific function returns EFI_UNSUPPORTED, then
PiSmmCpuDxeSmm falls back to the common function
ReadSaveStateRegister(), defined in file
"UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c".
Current ReadSaveStateRegister() in
UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c is trying to copy data
from I/O Memory Address for EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX,
PF will happen as SMM page table does not know and cover this
OS/Virtual Machine virtual address.
Same case is for SmmCpuFeaturesReadSaveStateRegister() in platform-
specific SmmCpuFeaturesLib instance if it has similar implementation
to read save state for EFI_SMM_SAVE_STATE_REGISTER_IO with
EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX.
Same case is for "ins", 'outs' and 'rep ins'.
So to fix the problem, this patch updates the code to only support
IN/OUT, but not INS/OUTS/REP INS/REP OUTS for SmmReadSaveState().
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Diffstat (limited to 'UefiCpuPkg/PiSmmCpuDxeSmm')
-rw-r--r-- | UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 17 |
1 files changed, 9 insertions, 8 deletions
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c index 26e365eabc..11102c9cbf 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c @@ -360,7 +360,6 @@ ReadSaveStateRegister ( UINT32 SmmRevId;
SMRAM_SAVE_STATE_IOMISC IoMisc;
EFI_SMM_SAVE_STATE_IO_INFO *IoInfo;
- VOID *IoMemAddr;
//
// Check for special EFI_SMM_SAVE_STATE_REGISTER_LMA
@@ -407,6 +406,14 @@ ReadSaveStateRegister ( }
//
+ // Only support IN/OUT, but not INS/OUTS/REP INS/REP OUTS.
+ //
+ if ((mSmmCpuIoType[IoMisc.Bits.Type] != EFI_SMM_SAVE_STATE_IO_TYPE_INPUT) &&
+ (mSmmCpuIoType[IoMisc.Bits.Type] != EFI_SMM_SAVE_STATE_IO_TYPE_OUTPUT)) {
+ return EFI_NOT_FOUND;
+ }
+
+ //
// Compute index for the I/O Length and I/O Type lookup tables
//
if (mSmmCpuIoWidth[IoMisc.Bits.Length].Width == 0 || mSmmCpuIoType[IoMisc.Bits.Type] == 0) {
@@ -425,13 +432,7 @@ ReadSaveStateRegister ( IoInfo->IoPort = (UINT16)IoMisc.Bits.Port;
IoInfo->IoWidth = mSmmCpuIoWidth[IoMisc.Bits.Length].IoWidth;
IoInfo->IoType = mSmmCpuIoType[IoMisc.Bits.Type];
- if (IoInfo->IoType == EFI_SMM_SAVE_STATE_IO_TYPE_INPUT || IoInfo->IoType == EFI_SMM_SAVE_STATE_IO_TYPE_OUTPUT) {
- ReadSaveStateRegister (CpuIndex, EFI_SMM_SAVE_STATE_REGISTER_RAX, mSmmCpuIoWidth[IoMisc.Bits.Length].Width, &IoInfo->IoData);
- }
- else {
- ReadSaveStateRegisterByIndex(CpuIndex, SMM_SAVE_STATE_REGISTER_IOMEMADDR_INDEX, sizeof(IoMemAddr), &IoMemAddr);
- CopyMem(&IoInfo->IoData, IoMemAddr, mSmmCpuIoWidth[IoMisc.Bits.Length].Width);
- }
+ ReadSaveStateRegister (CpuIndex, EFI_SMM_SAVE_STATE_REGISTER_RAX, mSmmCpuIoWidth[IoMisc.Bits.Length].Width, &IoInfo->IoData);
return EFI_SUCCESS;
}
|