mirror of https://github.com/acidanthera/audk.git
OvmfPkg: PlatformPei: protect SEC's GUIDed section handler table thru S3
OVMF's SecMain is unique in the sense that it links against the following two libraries *in combination*: - IntelFrameworkModulePkg/Library/LzmaCustomDecompressLib/ LzmaCustomDecompressLib.inf - MdePkg/Library/BaseExtractGuidedSectionLib/ BaseExtractGuidedSectionLib.inf The ExtractGuidedSectionLib library class allows decompressor modules to register themselves (keyed by GUID) with it, and it allows clients to decompress file sections with a registered decompressor module that matches the section's GUID. BaseExtractGuidedSectionLib is a library instance (of type BASE) for this library class. It has no constructor function. LzmaCustomDecompressLib is a compatible decompressor module (of type BASE). Its section type GUID is gLzmaCustomDecompressGuid == EE4E5898-3914-4259-9D6E-DC7BD79403CF When OVMF's SecMain module starts, the LzmaCustomDecompressLib constructor function is executed, which registers its LZMA decompressor with the above GUID, by calling into BaseExtractGuidedSectionLib: LzmaDecompressLibConstructor() [GuidedSectionExtraction.c] ExtractGuidedSectionRegisterHandlers() [BaseExtractGuidedSectionLib.c] GetExtractGuidedSectionHandlerInfo() PcdGet64 (PcdGuidedExtractHandlerTableAddress) -- NOTE THIS Later, during a normal (non-S3) boot, SecMain utilizes this decompressor to get information about, and to decompress, sections of the OVMF firmware image: SecCoreStartupWithStack() [OvmfPkg/Sec/SecMain.c] SecStartupPhase2() FindAndReportEntryPoints() FindPeiCoreImageBase() DecompressMemFvs() ExtractGuidedSectionGetInfo() [BaseExtractGuidedSectionLib.c] ExtractGuidedSectionDecode() [BaseExtractGuidedSectionLib.c] Notably, only the extraction depends on full-config-boot; the registration of LzmaCustomDecompressLib occurs unconditionally in the SecMain EFI binary, triggered by the library constructor function. This is where the bug happens. BaseExtractGuidedSectionLib maintains the table of GUIDed decompressors (section handlers) at a fixed memory location; selected by PcdGuidedExtractHandlerTableAddress (declared in MdePkg.dec). The default value of this PCD is 0x1000000 (16 MB). This causes SecMain to corrupt guest OS memory during S3, leading to random crashes. Compare the following two memory dumps, the first taken right before suspending, the second taken right after resuming a RHEL-7 guest: crash> rd -8 -p 1000000 0x50 1000000: c0 00 08 00 02 00 00 00 00 00 00 00 00 00 00 00 ................ 1000010: d0 33 0c 00 00 c9 ff ff c0 10 00 01 00 88 ff ff .3.............. 1000020: 0a 6d 57 32 0f 00 00 00 38 00 00 01 00 88 ff ff .mW2....8....... 1000030: 00 00 00 00 00 00 00 00 73 69 67 6e 61 6c 6d 6f ........signalmo 1000040: 64 75 6c 65 2e 73 6f 00 00 00 00 00 00 00 00 00 dule.so......... vs. crash> rd -8 -p 1000000 0x50 1000000: 45 47 53 49 01 00 00 00 20 00 00 01 00 00 00 00 EGSI.... ....... 1000010: 20 01 00 01 00 00 00 00 a0 01 00 01 00 00 00 00 ............... 1000020: 98 58 4e ee 14 39 59 42 9d 6e dc 7b d7 94 03 cf .XN..9YB.n.{.... 1000030: 00 00 00 00 00 00 00 00 73 69 67 6e 61 6c 6d 6f ........signalmo 1000040: 64 75 6c 65 2e 73 6f 00 00 00 00 00 00 00 00 00 dule.so......... The "EGSI" signature corresponds to EXTRACT_HANDLER_INFO_SIGNATURE declared in MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.c. Additionally, the gLzmaCustomDecompressGuid (quoted above) is visible at guest-phys offset 0x1000020. Fix the problem as follows: - Carve out 4KB from the 36KB gap that we currently have between PcdOvmfLockBoxStorageBase + PcdOvmfLockBoxStorageSize == 8220 KB and PcdOvmfSecPeiTempRamBase == 8256 KB. - Point PcdGuidedExtractHandlerTableAddress to 8220 KB (0x00807000). - Cover the area with an EfiACPIMemoryNVS type memalloc HOB, if S3 is supported and we're not currently resuming. The 4KB size that we pick is an upper estimate for BaseExtractGuidedSectionLib's internal storage size. The latter is calculated as follows (see GetExtractGuidedSectionHandlerInfo()): sizeof(EXTRACT_GUIDED_SECTION_HANDLER_INFO) + // 32 PcdMaximumGuidedExtractHandler * ( sizeof(GUID) + // 16 sizeof(EXTRACT_GUIDED_SECTION_DECODE_HANDLER) + // 8 sizeof(EXTRACT_GUIDED_SECTION_GET_INFO_HANDLER) // 8 ) OVMF sets PcdMaximumGuidedExtractHandler to 16 decimal (which is the MdePkg default too), yielding 32 + 16 * (16 + 8 + 8) == 544 bytes. Regarding the lifecycle of the new area: (a) when and how it is initialized after first boot of the VM The library linked into SecMain finds that the area lacks the signature. It initializes the signature, plus the rest of the structure. This is independent of S3 support. Consumption of the area is also limited to SEC (but consumption does depend on full-config-boot). (b) how it is protected from memory allocations during DXE It is not, in the general case; and we don't need to. Nothing else links against BaseExtractGuidedSectionLib; it's OK if DXE overwrites the area. (c) how it is protected from the OS When S3 is enabled, we cover it with AcpiNVS in InitializeRamRegions(). When S3 is not supported, the range is not protected. (d) how it is accessed on the S3 resume path Examined by the library linked into SecMain. Registrations update the table in-place (based on GUID matches). (e) how it is accessed on the warm reset path If S3 is enabled, then the OS won't damage the table (due to (c)), hence see (d). If S3 is unsupported, then the OS may or may not overwrite the signature. (It likely will.) This is identical to the pre-patch status. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15433 6f19259b-4bc3-4df7-8a09-765794883524
This commit is contained in:
parent
e9d19a80af
commit
ad43bc6b2e
|
@ -88,6 +88,7 @@
|
|||
gUefiOvmfPkgTokenSpaceGuid.PcdS3AcpiReservedMemoryBase|0x0|UINT32|0x17
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|0x0|UINT32|0x18
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize|0x0|UINT32|0x19
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize|0x0|UINT32|0x1a
|
||||
|
||||
[PcdsDynamic, PcdsDynamicEx]
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
|
||||
|
|
|
@ -141,6 +141,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.P
|
|||
0x006000|0x001000
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
|
||||
|
||||
0x007000|0x001000
|
||||
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
|
||||
|
||||
0x010000|0x008000
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
|
||||
|
||||
|
|
|
@ -141,6 +141,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.P
|
|||
0x006000|0x001000
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
|
||||
|
||||
0x007000|0x001000
|
||||
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
|
||||
|
||||
0x010000|0x008000
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
|
||||
|
||||
|
|
|
@ -141,6 +141,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.P
|
|||
0x006000|0x001000
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
|
||||
|
||||
0x007000|0x001000
|
||||
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
|
||||
|
||||
0x010000|0x008000
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
|
||||
|
||||
|
|
|
@ -205,6 +205,15 @@ InitializeRamRegions (
|
|||
EfiACPIMemoryNVS
|
||||
);
|
||||
|
||||
//
|
||||
// SEC stores its table of GUIDed section handlers here.
|
||||
//
|
||||
BuildMemoryAllocationHob (
|
||||
PcdGet64 (PcdGuidedExtractHandlerTableAddress),
|
||||
PcdGet32 (PcdGuidedExtractHandlerTableSize),
|
||||
EfiACPIMemoryNVS
|
||||
);
|
||||
|
||||
#ifdef MDE_CPU_X64
|
||||
//
|
||||
// Reserve the initial page tables built by the reset vector code.
|
||||
|
|
|
@ -72,7 +72,9 @@
|
|||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
|
||||
gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
|
||||
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize
|
||||
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
|
||||
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize
|
||||
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
|
||||
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
|
||||
|
|
Loading…
Reference in New Issue