]> git.proxmox.com Git - mirror_edk2.git/commitdiff
OvmfPkg: PlatformPei: protect SEC's GUIDed section handler table thru S3
authorLaszlo Ersek <lersek@redhat.com>
Sat, 5 Apr 2014 21:26:09 +0000 (21:26 +0000)
committerjljusten <jljusten@6f19259b-4bc3-4df7-8a09-765794883524>
Sat, 5 Apr 2014 21:26:09 +0000 (21:26 +0000)
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

OvmfPkg/OvmfPkg.dec
OvmfPkg/OvmfPkgIa32.fdf
OvmfPkg/OvmfPkgIa32X64.fdf
OvmfPkg/OvmfPkgX64.fdf
OvmfPkg/PlatformPei/MemDetect.c
OvmfPkg/PlatformPei/PlatformPei.inf

index aca9cb7cc747471bc764b2e5c5f96910518f9a74..c10948dc3bd005ae611436df09a333de36895a44 100644 (file)
@@ -88,6 +88,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdS3AcpiReservedMemoryBase|0x0|UINT32|0x17\r
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|0x0|UINT32|0x18\r
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize|0x0|UINT32|0x19\r
+  gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize|0x0|UINT32|0x1a\r
 \r
 [PcdsDynamic, PcdsDynamicEx]\r
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2\r
index 1f78b5bf92e51e4fa8b3a98856ba04c3c709844f..76b868dde4501ab2b3172091bcf8922b91484484 100644 (file)
@@ -141,6 +141,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.P
 0x006000|0x001000\r
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize\r
 \r
+0x007000|0x001000\r
+gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize\r
+\r
 0x010000|0x008000\r
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize\r
 \r
index e78995b32d350940ea5b3d4f4408a2a86034c93a..e037b7229055cab9b7ff6e0ecbf294f3b9d49478 100644 (file)
@@ -141,6 +141,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.P
 0x006000|0x001000\r
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize\r
 \r
+0x007000|0x001000\r
+gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize\r
+\r
 0x010000|0x008000\r
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize\r
 \r
index c5bc6ac169c06690c2d964b960bbbf88518d0205..b9e9c59facf9e16c5ca8af5560a8d52c60c9461d 100644 (file)
@@ -141,6 +141,9 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.P
 0x006000|0x001000\r
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize\r
 \r
+0x007000|0x001000\r
+gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize\r
+\r
 0x010000|0x008000\r
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize\r
 \r
index 15b279e4456c525ab18912bb4aafa096ad0204a3..4c22679c7af95aa4c67ea931f4fa7239eb29cd3e 100644 (file)
@@ -205,6 +205,15 @@ InitializeRamRegions (
       EfiACPIMemoryNVS\r
       );\r
 \r
+    //\r
+    // SEC stores its table of GUIDed section handlers here.\r
+    //\r
+    BuildMemoryAllocationHob (\r
+      PcdGet64 (PcdGuidedExtractHandlerTableAddress),\r
+      PcdGet32 (PcdGuidedExtractHandlerTableSize),\r
+      EfiACPIMemoryNVS\r
+      );\r
+\r
 #ifdef MDE_CPU_X64\r
     //\r
     // Reserve the initial page tables built by the reset vector code.\r
index 3b47bb70dd079ff3ac32dfbb5d25ba69b9eeda52..a5fa9b5004d7c837ef13f930681dae13ce34f3bd 100644 (file)
@@ -72,7 +72,9 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize\r
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase\r
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize\r
+  gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize\r
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize\r
+  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize\r