If OVMF was built with -D SMM_REQUIRE, that implies that the runtime OS is not trusted and we should defend against it tampering with the firmware's data.
One such datum is the PEI firmware volume (PEIFV). Normally PEIFV is decompressed on the first boot by SEC, then the OS preserves it across S3 suspend-resume cycles; at S3 resume SEC just reuses the originally decompressed PEIFV. However, if we don't trust the OS, then SEC must decompress PEIFV from the pristine flash every time, lest we execute OS-injected code or work with OS-injected data. Due to how FVMAIN_COMPACT is organized, we can't decompress just PEIFV; the decompression brings DXEFV with itself, plus it uses a temporary output buffer and a scratch buffer too, which even reach above the end of the finally installed DXEFV. For this reason we must keep away a non-malicious OS from DXEFV too, plus the memory up to PcdOvmfDecomprScratchEnd. The delay introduced by the LZMA decompression on S3 resume is negligible. If -D SMM_REQUIRE is not specified, then PcdSmmSmramRequire remains FALSE (from the DEC file), and then this patch has no effect (not counting some changed debug messages). If QEMU doesn't support S3 (or the user disabled it on the QEMU command line), then this patch has no effect also. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <[email protected]> --- OvmfPkg/PlatformPei/PlatformPei.inf | 4 +++ OvmfPkg/Sec/SecMain.inf | 3 +++ OvmfPkg/PlatformPei/Fv.c | 27 +++++++++++++++++++- OvmfPkg/PlatformPei/MemDetect.c | 11 +++++++- OvmfPkg/Sec/SecMain.c | 18 ++++++++++--- 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf index cb7d7dd..b9b0b20 100644 --- a/OvmfPkg/PlatformPei/PlatformPei.inf +++ b/OvmfPkg/PlatformPei/PlatformPei.inf @@ -75,6 +75,7 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecomprScratchEnd gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize @@ -87,6 +88,9 @@ [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress +[FeaturePcd] + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire + [Ppis] gEfiPeiMasterBootModePpiGuid diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf index 0bd9f83..c66c7fc 100644 --- a/OvmfPkg/Sec/SecMain.inf +++ b/OvmfPkg/Sec/SecMain.inf @@ -68,3 +68,6 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecomprScratchEnd + +[FeaturePcd] + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire diff --git a/OvmfPkg/PlatformPei/Fv.c b/OvmfPkg/PlatformPei/Fv.c index 3ed775c..7a89acd 100644 --- a/OvmfPkg/PlatformPei/Fv.c +++ b/OvmfPkg/PlatformPei/Fv.c @@ -32,6 +32,8 @@ PeiFvInitialization ( VOID ) { + BOOLEAN SecureS3Needed; + DEBUG ((EFI_D_INFO, "Platform PEI Firmware Volume Initialization\n")); // @@ -50,16 +52,39 @@ PeiFvInitialization ( // BuildFvHob (PcdGet32 (PcdOvmfDxeMemFvBase), PcdGet32 (PcdOvmfDxeMemFvSize)); + SecureS3Needed = mS3Supported && FeaturePcdGet (PcdSmmSmramRequire); + // // Create a memory allocation HOB for the DXE FV. // + // If "secure" S3 is needed, then SEC will decompress both PEI and DXE + // firmware volumes at S3 resume too, hence we need to keep away the OS from + // DXEFV as well. Otherwise we only need to keep away DXE itself from the + // DXEFV area. + // BuildMemoryAllocationHob ( PcdGet32 (PcdOvmfDxeMemFvBase), PcdGet32 (PcdOvmfDxeMemFvSize), - EfiBootServicesData + SecureS3Needed ? EfiACPIMemoryNVS : EfiBootServicesData ); // + // Additionally, said decompression will use temporary memory above the end + // of DXEFV, so let's keep away the OS from there too. + // + if (SecureS3Needed) { + UINT32 DxeMemFvEnd; + + DxeMemFvEnd = PcdGet32 (PcdOvmfDxeMemFvBase) + + PcdGet32 (PcdOvmfDxeMemFvSize); + BuildMemoryAllocationHob ( + DxeMemFvEnd, + PcdGet32 (PcdOvmfDecomprScratchEnd) - DxeMemFvEnd, + EfiACPIMemoryNVS + ); + } + + // // Let PEI know about the DXE FV so it can find the DXE Core // PeiServicesInstallFvInfoPpi ( diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c index 612bb4a..649b484 100644 --- a/OvmfPkg/PlatformPei/MemDetect.c +++ b/OvmfPkg/PlatformPei/MemDetect.c @@ -222,7 +222,16 @@ PublishPeiMemory ( // // Determine the range of memory to use during PEI // - MemoryBase = PcdGet32 (PcdOvmfDxeMemFvBase) + PcdGet32 (PcdOvmfDxeMemFvSize); + // Technically we could lay the permanent PEI RAM over SEC's temporary + // decompression and scratch buffer even if "secure S3" is needed, since + // their lifetimes don't overlap. However, PeiFvInitialization() will cover + // RAM up to PcdOvmfDecomprScratchEnd with an EfiACPIMemoryNVS memory + // allocation HOB, and other allocations served from the permanent PEI RAM + // shouldn't overlap with that HOB. + // + MemoryBase = mS3Supported && FeaturePcdGet (PcdSmmSmramRequire) ? + PcdGet32 (PcdOvmfDecomprScratchEnd) : + PcdGet32 (PcdOvmfDxeMemFvBase) + PcdGet32 (PcdOvmfDxeMemFvSize); MemorySize = LowerMemorySize - MemoryBase; if (MemorySize > PeiMemoryCap) { MemoryBase = LowerMemorySize - PeiMemoryCap; diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index 348d2b3..99dd6db 100644 --- a/OvmfPkg/Sec/SecMain.c +++ b/OvmfPkg/Sec/SecMain.c @@ -538,13 +538,25 @@ FindPeiCoreImageBase ( OUT EFI_PHYSICAL_ADDRESS *PeiCoreImageBase ) { + BOOLEAN S3Resume; + *PeiCoreImageBase = 0; - if (IsS3Resume ()) { - DEBUG ((EFI_D_VERBOSE, "SEC: S3 resume\n")); + S3Resume = IsS3Resume (); + if (S3Resume && !FeaturePcdGet (PcdSmmSmramRequire)) { + // + // A malicious runtime OS may have injected something into our previously + // decoded PEI FV, but we don't care about that unless SMM/SMRAM is required. + // + DEBUG ((EFI_D_VERBOSE, "SEC: S3 resume (insecure)\n")); GetS3ResumePeiFv (BootFv); } else { - DEBUG ((EFI_D_VERBOSE, "SEC: Normal boot\n")); + // + // We're either not resuming, or resuming "securely" -- we'll decompress + // both PEI FV and DXE FV from pristine flash. + // + DEBUG ((EFI_D_VERBOSE, "SEC: %a\n", + S3Resume ? "S3 resume (hopefully secure)" : "Normal boot")); FindMainFv (BootFv); DecompressMemFvs (BootFv); -- 1.8.3.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
