[edk2,2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once

Permit ProcessCapsules () to be called only a single time, after
EndOfDxe. This allows platforms that are able to update system
firmware after EndOfDxe (e.g., because the flash ROM is not locked
down) to do so at a time when a non-trusted console is up and running,
and progress can be reported to the user.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

-- 
2.17.0

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

Without the patch, PopulateCapsuleInConfigurationTable is only run at first round.
With the patch, PopulateCapsuleInConfigurationTable is only run at last round.

Is that expected?

Jiewen, could you help check whether the patch meets the original design purpose or any security concern?


Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 

Sent: Thursday, June 7, 2018 7:08 PM
To: edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once

Permit ProcessCapsules () to be called only a single time, after EndOfDxe. This allows platforms that are able to update system firmware after EndOfDxe (e.g., because the flash ROM is not locked
down) to do so at a time when a non-trusted console is up and running, and progress can be reported to the user.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
index 26ca4e295f20..52691fa68be4 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
@@ -100,6 +100,7 @@ IsValidCapsuleHeader (
 
 extern BOOLEAN                   mDxeCapsuleLibEndOfDxe;
 BOOLEAN                          mNeedReset;
+BOOLEAN                          mFirstRound = TRUE;
 
 VOID                        **mCapsulePtr;
 EFI_STATUS                  *mCapsuleStatusArray;
@@ -364,8 +365,10 @@ PopulateCapsuleInConfigurationTable (
 
   Each individual capsule result is recorded in capsule record variable.
 
-  @param[in]  FirstRound         TRUE:  First round. Need skip the FMP capsules with non zero EmbeddedDriverCount.
-                                 FALSE: Process rest FMP capsules.
+  @param[in]  LastRound          FALSE:  First of multiple rounds. Need skip the
+                                         FMP capsules with non zero
+                                         EmbeddedDriverCount.
+                                 TRUE:   Process rest FMP capsules.
 
   @retval EFI_SUCCESS             There is no error when processing capsules.
   @retval EFI_OUT_OF_RESOURCES    No enough resource to process capsules.
@@ -373,7 +376,7 @@ PopulateCapsuleInConfigurationTable (  **/  EFI_STATUS  ProcessTheseCapsules (
-  IN BOOLEAN  FirstRound
+  IN BOOLEAN  LastRound
   )
 {
   EFI_STATUS                  Status;
@@ -384,8 +387,9 @@ ProcessTheseCapsules (
 
   REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeProcessCapsulesBegin)));
 
-  if (FirstRound) {
+  if (mFirstRound) {
     InitCapsulePtr ();
+    mFirstRound = FALSE;
   }
 
   if (mCapsuleTotalNumber == 0) {
@@ -404,7 +408,7 @@ ProcessTheseCapsules (
   // Check the capsule flags,if contains CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE, install
   // capsuleTable to configure table with EFI_CAPSULE_GUID
   //
-  if (FirstRound) {
+  if (LastRound) {
     PopulateCapsuleInConfigurationTable ();
   }
 
@@ -453,7 +457,7 @@ ProcessTheseCapsules (
         continue;
       }
 
-      if ((!FirstRound) || (EmbeddedDriverCount == 0)) {
+      if (LastRound || (EmbeddedDriverCount == 0)) {
         DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", CapsuleHeader));
         Status = ProcessCapsuleImage (CapsuleHeader);
         mCapsuleStatusArray [Index] = Status; @@ -546,7 +550,7 @@ ProcessCapsules (
   EFI_STATUS                    Status;
 
   if (!mDxeCapsuleLibEndOfDxe) {
-    Status = ProcessTheseCapsules(TRUE);
+    Status = ProcessTheseCapsules(FALSE);
 
     //
     // Reboot System if and only if all capsule processed.
@@ -556,7 +560,7 @@ ProcessCapsules (
       DoResetSystem();
     }
   } else {
-    Status = ProcessTheseCapsules(FALSE);
+    Status = ProcessTheseCapsules(TRUE);
     //
     // Reboot System if required after all capsule processed
     //
--
2.17.0

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

On 8 June 2018 at 04:57, Zeng, Star <star.zeng@intel.com> wrote:
> Without the patch, PopulateCapsuleInConfigurationTable is only run at first round.

> With the patch, PopulateCapsuleInConfigurationTable is only run at last round.

>

> Is that expected?

>


Yes. Otherwise, I need two global BOOLEANs to keep track of the state.

However, I just noticed that we may now end up in a situation where
PopulateCapsuleInConfigurationTable() never gets called if all
capsules are processed in the first pass.

I will fix and resend.

> Jiewen, could you help check whether the patch meets the original design purpose or any security concern?

>

>

> Thanks,

> Star

> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Thursday, June 7, 2018 7:08 PM

> To: edk2-devel@lists.01.org

> Cc: leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Subject: [PATCH 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once

>

> Permit ProcessCapsules () to be called only a single time, after EndOfDxe. This allows platforms that are able to update system firmware after EndOfDxe (e.g., because the flash ROM is not locked

> down) to do so at a time when a non-trusted console is up and running, and progress can be reported to the user.

>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 20 ++++++++++++--------

>  1 file changed, 12 insertions(+), 8 deletions(-)

>

> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c

> index 26ca4e295f20..52691fa68be4 100644

> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c

> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c

> @@ -100,6 +100,7 @@ IsValidCapsuleHeader (

>

>  extern BOOLEAN                   mDxeCapsuleLibEndOfDxe;

>  BOOLEAN                          mNeedReset;

> +BOOLEAN                          mFirstRound = TRUE;

>

>  VOID                        **mCapsulePtr;

>  EFI_STATUS                  *mCapsuleStatusArray;

> @@ -364,8 +365,10 @@ PopulateCapsuleInConfigurationTable (

>

>    Each individual capsule result is recorded in capsule record variable.

>

> -  @param[in]  FirstRound         TRUE:  First round. Need skip the FMP capsules with non zero EmbeddedDriverCount.

> -                                 FALSE: Process rest FMP capsules.

> +  @param[in]  LastRound          FALSE:  First of multiple rounds. Need skip the

> +                                         FMP capsules with non zero

> +                                         EmbeddedDriverCount.

> +                                 TRUE:   Process rest FMP capsules.

>

>    @retval EFI_SUCCESS             There is no error when processing capsules.

>    @retval EFI_OUT_OF_RESOURCES    No enough resource to process capsules.

> @@ -373,7 +376,7 @@ PopulateCapsuleInConfigurationTable (  **/  EFI_STATUS  ProcessTheseCapsules (

> -  IN BOOLEAN  FirstRound

> +  IN BOOLEAN  LastRound

>    )

>  {

>    EFI_STATUS                  Status;

> @@ -384,8 +387,9 @@ ProcessTheseCapsules (

>

>    REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeProcessCapsulesBegin)));

>

> -  if (FirstRound) {

> +  if (mFirstRound) {

>      InitCapsulePtr ();

> +    mFirstRound = FALSE;

>    }

>

>    if (mCapsuleTotalNumber == 0) {

> @@ -404,7 +408,7 @@ ProcessTheseCapsules (

>    // Check the capsule flags,if contains CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE, install

>    // capsuleTable to configure table with EFI_CAPSULE_GUID

>    //

> -  if (FirstRound) {

> +  if (LastRound) {

>      PopulateCapsuleInConfigurationTable ();

>    }

>

> @@ -453,7 +457,7 @@ ProcessTheseCapsules (

>          continue;

>        }

>

> -      if ((!FirstRound) || (EmbeddedDriverCount == 0)) {

> +      if (LastRound || (EmbeddedDriverCount == 0)) {

>          DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", CapsuleHeader));

>          Status = ProcessCapsuleImage (CapsuleHeader);

>          mCapsuleStatusArray [Index] = Status; @@ -546,7 +550,7 @@ ProcessCapsules (

>    EFI_STATUS                    Status;

>

>    if (!mDxeCapsuleLibEndOfDxe) {

> -    Status = ProcessTheseCapsules(TRUE);

> +    Status = ProcessTheseCapsules(FALSE);

>

>      //

>      // Reboot System if and only if all capsule processed.

> @@ -556,7 +560,7 @@ ProcessCapsules (

>        DoResetSystem();

>      }

>    } else {

> -    Status = ProcessTheseCapsules(FALSE);

> +    Status = ProcessTheseCapsules(TRUE);

>      //

>      // Reboot System if required after all capsule processed

>      //

> --

> 2.17.0

>

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