BaseExtractGuidedSectionLib uses a table at the static physical address
PcdGuidedExtractHandlerTableAddress, and modules that are linked against
BaseExtractGuidedSectionLib are expected to work together on that table.
Namely, some modules can register handlers for GUIDed sections, some other
modules can decode such sections with the pre-registered handlers. The
table carries persistent information between these modules.

BaseExtractGuidedSectionLib checks a table signature whenever it is used
(by whichever module that is linked against it), and at the first use
(identified by a signature mismatch) it initializes the table.

One of the module types that BaseExtractGuidedSectionLib can be used with
is SEC, if the SEC module in question runs with the platform's RAM already
available.

In such cases the question emerges whether the initial contents of the RAM
(ie. contents that predate the very first signature check) can be trusted.
Normally RAM starts out with all zeroes (leading to a signature mismatch
on the first check); however a malicious runtime OS can populate the area
with some payload, then force a warm platform reset or an S3
suspend-and-resume. In such cases the signature check in the SEC module
might not fire, and ExtractGuidedSectionDecode() might run code injected
by the runtime OS, as part of SEC (ie. with high privileges).

Allow DSC files to force specific linked instances of
BaseExtractGuidedSectionLib to reinitialize the table.

Cc: Michael D Kinney <michael.d.kin...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---

Notes:
    In the open source edk2 tree, no platform other than OvmfPkg uses
    BaseExtractGuidedSectionLib, and OvmfPkg doesn't claim to be secure
    against the described kind of attack (ie. code or data injection by a
    malicious runtime OS before warm reset or S3) anyway. Hence as far as
    the open source tree is concerned, the vulnerability is purely
    theoretical.
    
    However, vendors might have closed source platforms that use
    BaseExtractGuidedSectionLib in SEC, while also supporting Secure Boot.
    For them the vulnerability might be real.
    
    In any case, I didn't try to come up with an actual exploit.
    
    I posted this patch to <tianocore-secur...@lists.sourceforge.net> on Apr
    23rd, and haven't got any feedback since. So I'm including it here.

 MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf |  
4 +++
 MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.c   | 
31 ++++++++++++++++++++
 MdePkg/MdePkg.dec                                                          | 
13 ++++++++
 3 files changed, 48 insertions(+)

diff --git 
a/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf 
b/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
index 1bd245e..6fc528a 100644
--- a/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
+++ b/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
@@ -8,6 +8,7 @@
 #  the implementation of runtime services, because this BASE library instance 
doesn't 
 #  convert the address pointed by PcdGuidedExtractHandlerTableAddress to the 
virtual address.
 #  
+#  Copyright (C) 2015, Red Hat, Inc.<BR>
 #  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
@@ -27,6 +28,7 @@ [Defines]
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = ExtractGuidedSectionLib
+  CONSTRUCTOR                    = BaseExtractGuidedSectionLibInit
 
 #
 # The following information is for reference only and not required by the 
build tools.
@@ -49,3 +51,5 @@ [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler         ## CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress    ## CONSUMES
 
+[FeaturePcd]
+  gEfiMdePkgTokenSpaceGuid.PcdBaseExtractGuidedSectionLibForceInit ## CONSUMES
diff --git 
a/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.c 
b/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.c
index 3a12cb1..d4dfb47 100644
--- a/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.c
+++ b/MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.c
@@ -1,6 +1,7 @@
 /** @file
   Provide generic extract guided section functions.
 
+  Copyright (C) 2015, Red Hat, Inc.<BR>
   Copyright (c) 2007 - 2011, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -30,6 +31,36 @@ typedef struct {
 } EXTRACT_GUIDED_SECTION_HANDLER_INFO;
 
 /**
+  The library constructor is generally a no-op. However, when linked into a SEC
+  module that runs with RAM available, the platform description file should
+  enable the constructor to force a table reinitialization in
+  GetExtractGuidedSectionHandlerInfo() below, if the memory that SEC runs on is
+  not trusted.
+**/
+RETURN_STATUS
+EFIAPI
+BaseExtractGuidedSectionLibInit (
+  VOID
+  )
+{
+  EXTRACT_GUIDED_SECTION_HANDLER_INFO *HandlerInfo;
+
+  if (!FeaturePcdGet (PcdBaseExtractGuidedSectionLibForceInit)) {
+    return RETURN_SUCCESS;
+  }
+
+  HandlerInfo = (VOID*)(UINTN)PcdGet64 (PcdGuidedExtractHandlerTableAddress);
+  if (HandlerInfo == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  HandlerInfo->Signature = 0;
+  DEBUG ((DEBUG_VERBOSE, "%a: cleared handler table signature\n",
+    __FUNCTION__));
+  return RETURN_SUCCESS;
+}
+
+/**
   HandlerInfo table address is set by PcdGuidedExtractHandlerTableAddress, 
which is used to store 
   the registered guid and Handler list. When it is initialized, it will be 
directly returned. 
   Or, HandlerInfo table will be initialized in this function.
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 598a6d0..5dc11b3 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1631,6 +1631,19 @@ [PcdsFeatureFlag]
   # @Prompt Validate ORDERED_COLLECTION structure
   
gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection|FALSE|BOOLEAN|0x0000002a
 
+  ## If TRUE, then the constructor of BaseExtractGuidedSectionLib will force a
+  #  reinitialization of the table of GUIDed section handlers, located at
+  #  PcdGuidedExtractHandlerTableAddress. This is useful when
+  #  BaseExtractGuidedSectionLib is linked into a SEC module that runs with RAM
+  #  ready, and the prior memory contents at
+  #  PcdGuidedExtractHandlerTableAddress are untrusted.
+  #
+  #  The PCD should be left at FALSE generally, and overridden only for
+  #  individual SEC modules, with <PcdsFeatureFlag> stanzas in the [Components]
+  #  sections of DSC files.
+  #  @Prompt Force table initialization in BaseExtractGuidedSectionLib
+  
gEfiMdePkgTokenSpaceGuid.PcdBaseExtractGuidedSectionLibForceInit|FALSE|BOOLEAN|0x0000002d
+
 [PcdsFixedAtBuild]
   ## Status code value for indicating a watchdog timer has expired.
   # EFI_COMPUTING_UNIT_HOST_PROCESSOR | EFI_CU_HP_EC_TIMER_EXPIRED
-- 
1.8.3.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to