public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/2] MdeModulePkg PeiCore: Signed GUIDED section may not be dispatched
@ 2017-02-09  4:14 Liming Gao
  2017-02-09  4:14 ` [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation Liming Gao
  2017-02-09  4:14 ` [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED Liming Gao
  0 siblings, 2 replies; 5+ messages in thread
From: Liming Gao @ 2017-02-09  4:14 UTC (permalink / raw)
  To: edk2-devel

https://bugzilla.tianocore.org/show_bug.cgi?id=365

When RSA2048 GUIDED section has SIGNED attribute only without PROCESSED_REQUIRED 
attribute, it will not be processed correctly once RSA2048 GUIDED extraction 
service is dispatcher later, because PeiCore cache GUIDED section with 
EFI_AUTH_STATUS_NOT_TESTED.

Here is the failure case. RSA Extraction Service is compressed. DxeIpl installs
the decompress service. On the first round dispatcher, FVMAIN is cached with 
EFI_AUTH_STATUS_NOT_TESTED. It can't be dispatched again. 

INF RuleOverride = LzmaCompress MdeModulePkg/../SectionExtractionPei.inf
FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
  SECTION GUIDED A7717414-C616-4977-9420-844712A735BF AUTH_STATUS_VALID = TRUE
    SECTION FV_IMAGE = FVMAIN
  }
}
INF  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf

Liming Gao (2):
  MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security
    violation
  MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED

 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c |  9 ++++++++-
 MdeModulePkg/Core/Pei/FwVol/FwVol.c           | 24 +++++++++++++-----------
 2 files changed, 21 insertions(+), 12 deletions(-)

-- 
2.8.0.windows.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation
  2017-02-09  4:14 [Patch 0/2] MdeModulePkg PeiCore: Signed GUIDED section may not be dispatched Liming Gao
@ 2017-02-09  4:14 ` Liming Gao
  2017-02-09  4:44   ` Zeng, Star
  2017-02-09  4:14 ` [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED Liming Gao
  1 sibling, 1 reply; 5+ messages in thread
From: Liming Gao @ 2017-02-09  4:14 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng

When PEIM is security violation, its matched extraction ppi may not be
installed. So, its PeimNeedingDispatch will still reset to TRUE.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 3934ed0..ff43a90 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1,7 +1,7 @@
 /** @file
   EFI PEI Core dispatch services
   
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
@@ -1111,6 +1111,13 @@ PeiDispatcher (
                   PeimEntryPoint = (EFI_PEIM_ENTRY_POINT2)(UINTN)EntryPoint;
                   PeimEntryPoint (PeimFileHandle, (const EFI_PEI_SERVICES **) PeiServices);
                   Private->PeimDispatchOnThisPass = TRUE;
+                } else {
+                  //
+                  // The related GuidedSectionExtraction PPI for the
+                  // signed PEIM image section may be installed in the rest
+                  // of this do-while loop, so need to make another pass.
+                  //
+                  Private->PeimNeedingDispatch = TRUE;
                 }
 
                 REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
-- 
2.8.0.windows.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED
  2017-02-09  4:14 [Patch 0/2] MdeModulePkg PeiCore: Signed GUIDED section may not be dispatched Liming Gao
  2017-02-09  4:14 ` [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation Liming Gao
@ 2017-02-09  4:14 ` Liming Gao
  2017-02-09  4:40   ` Zeng, Star
  1 sibling, 1 reply; 5+ messages in thread
From: Liming Gao @ 2017-02-09  4:14 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng

If GUIDED section authentication has EFI_AUTH_STATUS_NOT_TESTED, its
matched extraction ppi may not be installed. So, don't cache its data.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Core/Pei/FwVol/FwVol.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
index 8d07bd0..0bbb86d 100644
--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
@@ -2,7 +2,7 @@
   Pei Core Firmware File System service routines.
   
 Copyright (c) 2015 HP Development Company, L.P.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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         
 which accompanies this distribution.  The full text of the license may be found at        
@@ -913,17 +913,19 @@ ProcessSection (
         }
 
         if (!EFI_ERROR (Status)) {
-          //
-          // Update cache section data.
-          //
-          if (PrivateData->CacheSection.AllSectionCount < CACHE_SETION_MAX_NUMBER) {
-            PrivateData->CacheSection.AllSectionCount ++;
+          if ((Authentication & EFI_AUTH_STATUS_NOT_TESTED) == 0) {
+            //
+            // Update cache section data.
+            //
+            if (PrivateData->CacheSection.AllSectionCount < CACHE_SETION_MAX_NUMBER) {
+              PrivateData->CacheSection.AllSectionCount ++;
+            }
+            PrivateData->CacheSection.Section [PrivateData->CacheSection.SectionIndex]     = Section;
+            PrivateData->CacheSection.SectionData [PrivateData->CacheSection.SectionIndex] = PpiOutput;
+            PrivateData->CacheSection.SectionSize [PrivateData->CacheSection.SectionIndex] = PpiOutputSize;
+            PrivateData->CacheSection.AuthenticationStatus [PrivateData->CacheSection.SectionIndex] = Authentication;
+            PrivateData->CacheSection.SectionIndex = (PrivateData->CacheSection.SectionIndex + 1)%CACHE_SETION_MAX_NUMBER;
           }
-          PrivateData->CacheSection.Section [PrivateData->CacheSection.SectionIndex]     = Section;
-          PrivateData->CacheSection.SectionData [PrivateData->CacheSection.SectionIndex] = PpiOutput;
-          PrivateData->CacheSection.SectionSize [PrivateData->CacheSection.SectionIndex] = PpiOutputSize;
-          PrivateData->CacheSection.AuthenticationStatus [PrivateData->CacheSection.SectionIndex] = Authentication;
-          PrivateData->CacheSection.SectionIndex = (PrivateData->CacheSection.SectionIndex + 1)%CACHE_SETION_MAX_NUMBER;
 
           TempAuthenticationStatus = 0;
           Status = ProcessSection (
-- 
2.8.0.windows.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED
  2017-02-09  4:14 ` [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED Liming Gao
@ 2017-02-09  4:40   ` Zeng, Star
  0 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2017-02-09  4:40 UTC (permalink / raw)
  To: Gao, Liming, edk2-devel@lists.01.org; +Cc: Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: Gao, Liming 
Sent: Thursday, February 9, 2017 12:15 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED

If GUIDED section authentication has EFI_AUTH_STATUS_NOT_TESTED, its matched extraction ppi may not be installed. So, don't cache its data.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Core/Pei/FwVol/FwVol.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
index 8d07bd0..0bbb86d 100644
--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
@@ -2,7 +2,7 @@
   Pei Core Firmware File System service routines.
   
 Copyright (c) 2015 HP Development Company, L.P.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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         
 which accompanies this distribution.  The full text of the license may be found at        
@@ -913,17 +913,19 @@ ProcessSection (
         }
 
         if (!EFI_ERROR (Status)) {
-          //
-          // Update cache section data.
-          //
-          if (PrivateData->CacheSection.AllSectionCount < CACHE_SETION_MAX_NUMBER) {
-            PrivateData->CacheSection.AllSectionCount ++;
+          if ((Authentication & EFI_AUTH_STATUS_NOT_TESTED) == 0) {
+            //
+            // Update cache section data.
+            //
+            if (PrivateData->CacheSection.AllSectionCount < CACHE_SETION_MAX_NUMBER) {
+              PrivateData->CacheSection.AllSectionCount ++;
+            }
+            PrivateData->CacheSection.Section [PrivateData->CacheSection.SectionIndex]     = Section;
+            PrivateData->CacheSection.SectionData [PrivateData->CacheSection.SectionIndex] = PpiOutput;
+            PrivateData->CacheSection.SectionSize [PrivateData->CacheSection.SectionIndex] = PpiOutputSize;
+            PrivateData->CacheSection.AuthenticationStatus [PrivateData->CacheSection.SectionIndex] = Authentication;
+            PrivateData->CacheSection.SectionIndex = 
+ (PrivateData->CacheSection.SectionIndex + 1)%CACHE_SETION_MAX_NUMBER;
           }
-          PrivateData->CacheSection.Section [PrivateData->CacheSection.SectionIndex]     = Section;
-          PrivateData->CacheSection.SectionData [PrivateData->CacheSection.SectionIndex] = PpiOutput;
-          PrivateData->CacheSection.SectionSize [PrivateData->CacheSection.SectionIndex] = PpiOutputSize;
-          PrivateData->CacheSection.AuthenticationStatus [PrivateData->CacheSection.SectionIndex] = Authentication;
-          PrivateData->CacheSection.SectionIndex = (PrivateData->CacheSection.SectionIndex + 1)%CACHE_SETION_MAX_NUMBER;
 
           TempAuthenticationStatus = 0;
           Status = ProcessSection (
--
2.8.0.windows.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation
  2017-02-09  4:14 ` [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation Liming Gao
@ 2017-02-09  4:44   ` Zeng, Star
  0 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2017-02-09  4:44 UTC (permalink / raw)
  To: Gao, Liming, edk2-devel@lists.01.org; +Cc: Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com> to this change.

How about to also move the code blocks below into the "if (Status != EFI_SECURITY_VIOLATION) {" to follow PI Spec?

                PERF_START (PeimFileHandle, "PEIM", NULL, 0);
                REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
                  EFI_PROGRESS_CODE,
                  (EFI_SOFTWARE_PEI_CORE | EFI_SW_PC_INIT_BEGIN),
                  (VOID *)(&PeimFileHandle),
                  sizeof (PeimFileHandle)
                  );

                REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
                  EFI_PROGRESS_CODE,
                  (EFI_SOFTWARE_PEI_CORE | EFI_SW_PC_INIT_END),
                  (VOID *)(&PeimFileHandle),
                  sizeof (PeimFileHandle)
                  );
                PERF_END (PeimFileHandle, "PEIM", NULL, 0);

PI Spec:
EFI_SW_PC_INIT_BEGIN
Initializing software module by using StartImage() or an equivalent PEI service.


Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Thursday, February 9, 2017 12:15 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation

When PEIM is security violation, its matched extraction ppi may not be installed. So, its PeimNeedingDispatch will still reset to TRUE.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 3934ed0..ff43a90 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1,7 +1,7 @@
 /** @file
   EFI PEI Core dispatch services
   
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>  This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License @@ -1111,6 +1111,13 @@ PeiDispatcher (
                   PeimEntryPoint = (EFI_PEIM_ENTRY_POINT2)(UINTN)EntryPoint;
                   PeimEntryPoint (PeimFileHandle, (const EFI_PEI_SERVICES **) PeiServices);
                   Private->PeimDispatchOnThisPass = TRUE;
+                } else {
+                  //
+                  // The related GuidedSectionExtraction PPI for the
+                  // signed PEIM image section may be installed in the rest
+                  // of this do-while loop, so need to make another pass.
+                  //
+                  Private->PeimNeedingDispatch = TRUE;
                 }
 
                 REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
--
2.8.0.windows.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-02-09  4:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-09  4:14 [Patch 0/2] MdeModulePkg PeiCore: Signed GUIDED section may not be dispatched Liming Gao
2017-02-09  4:14 ` [Patch 1/2] MdeModulePkg PeiCore: Reset PeimNeedingDispatch when its security violation Liming Gao
2017-02-09  4:44   ` Zeng, Star
2017-02-09  4:14 ` [Patch 2/2] MdeModulePkg PeiCore: Don't cache GUIDED section with AUTH_NOT_TESTED Liming Gao
2017-02-09  4:40   ` Zeng, Star

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox