From cf574f0a1838665498e43e9c029d8c1211cbbfa3 Mon Sep 17 00:00:00 2001 From: Star Zeng Date: Tue, 19 Dec 2017 09:59:50 +0800 Subject: [PATCH] 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 Cc: Ray Ni Cc: Laszlo Ersek Signed-off-by: Star Zeng Reviewed-by: Laszlo Ersek --- UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 17 +++++++++-------- 1 file 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 @@ -406,6 +405,14 @@ ReadSaveStateRegister ( return EFI_NOT_FOUND; } + // + // 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 // @@ -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; }