[PATCH 0/2] Variable: Make sure no more than one Variable HOB

[PATCH 0/2] Variable: Make sure no more than one Variable HOB

[Star Zeng]

Star Zeng (2):
  MdeModulePkg Variable: Abstract GetHobVariableStore function
  MdeModulePkg Variable: Make sure no more than one Variable HOB

 MdeModulePkg/Universal/Variable/Pei/Variable.c     |  64 ++++++++---
 .../Universal/Variable/RuntimeDxe/Variable.c       | 117 +++++++++++++++------
 2 files changed, 139 insertions(+), 42 deletions(-)

-- 
2.7.0.windows.1

[PATCH 1/2] MdeModulePkg Variable: Abstract GetHobVariableStore function

[Star Zeng]

Move getting HOB variable store code logic to a separated
GetHobVariableStore function.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Universal/Variable/Pei/Variable.c     | 40 +++++++---
 .../Universal/Variable/RuntimeDxe/Variable.c       | 93 +++++++++++++++-------
 2 files changed, 91 insertions(+), 42 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index 4060f4711dd0..1bcab3b770a5 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -2,7 +2,7 @@
   Implement ReadOnly Variable Services required by PEIM and install
   PEI ReadOnly Varaiable2 PPI. These services operates the non volatile storage space.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, 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
@@ -496,6 +496,31 @@ CompareWithValidVariable (
 }
 
 /**
+  Get HOB variable store.
+
+**/
+VOID
+GetHobVariableStore (
+  OUT VARIABLE_STORE_INFO        *StoreInfo,
+  OUT VARIABLE_STORE_HEADER      **VariableStoreHeader
+  )
+{
+  EFI_HOB_GUID_TYPE              *GuidHob;
+
+  GuidHob = GetFirstGuidHob (&gEfiAuthenticatedVariableGuid);
+  if (GuidHob != NULL) {
+    *VariableStoreHeader = (VARIABLE_STORE_HEADER *) GET_GUID_HOB_DATA (GuidHob);
+    StoreInfo->AuthFlag = TRUE;
+  } else {
+    GuidHob = GetFirstGuidHob (&gEfiVariableGuid);
+    if (GuidHob != NULL) {
+      *VariableStoreHeader = (VARIABLE_STORE_HEADER *) GET_GUID_HOB_DATA (GuidHob);
+      StoreInfo->AuthFlag = FALSE;
+    }
+  }
+}
+
+/**
   Return the variable store header and the store info based on the Index.
 
   @param Type       The type of the variable store.
@@ -523,17 +548,8 @@ GetVariableStore (
   VariableStoreHeader = NULL;
   switch (Type) {
     case VariableStoreTypeHob:
-      GuidHob = GetFirstGuidHob (&gEfiAuthenticatedVariableGuid);
-      if (GuidHob != NULL) {
-        VariableStoreHeader = (VARIABLE_STORE_HEADER *) GET_GUID_HOB_DATA (GuidHob);
-        StoreInfo->AuthFlag = TRUE;
-      } else {
-        GuidHob = GetFirstGuidHob (&gEfiVariableGuid);
-        if (GuidHob != NULL) {
-          VariableStoreHeader = (VARIABLE_STORE_HEADER *) GET_GUID_HOB_DATA (GuidHob);
-          StoreInfo->AuthFlag = FALSE;
-        }
-      }
+      GetHobVariableStore (StoreInfo, &VariableStoreHeader);
+
       break;
 
     case VariableStoreTypeNv:
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 835a66bc3bc8..fca8d5380924 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -4169,6 +4169,64 @@ ConvertNormalVarStorageToAuthVarStorage (
   mVariableModuleGlobal->VariableGlobal.AuthFormat = TRUE;
   return AuthVarStorage;
 }
+
+/**
+  Get HOB variable store.
+
+  @param[out] VariableGuid      NV variable store signature.
+
+  @retval EFI_SUCCESS           Function successfully executed.
+  @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resource.
+
+**/
+EFI_STATUS
+GetHobVariableStore (
+  IN EFI_GUID                   *VariableGuid
+  )
+{
+  VARIABLE_STORE_HEADER         *VariableStoreHeader;
+  UINT64                        VariableStoreLength;
+  EFI_HOB_GUID_TYPE             *GuidHob;
+  BOOLEAN                       NeedConvertNormalToAuth;
+
+  //
+  // Combinations supported:
+  // 1. Normal NV variable store +
+  //    Normal HOB variable store
+  // 2. Auth NV variable store +
+  //    Auth HOB variable store
+  // 3. Auth NV variable store +
+  //    Normal HOB variable store (code will convert it to Auth Format)
+  //
+  NeedConvertNormalToAuth = FALSE;
+  GuidHob = GetFirstGuidHob (VariableGuid);
+  if (GuidHob == NULL && VariableGuid == &gEfiAuthenticatedVariableGuid) {
+    //
+    // Try getting it from normal variable HOB
+    //
+    GuidHob = GetFirstGuidHob (&gEfiVariableGuid);
+    NeedConvertNormalToAuth = TRUE;
+  }
+  if (GuidHob != NULL) {
+    VariableStoreHeader = GET_GUID_HOB_DATA (GuidHob);
+    VariableStoreLength = GuidHob->Header.HobLength - sizeof (EFI_HOB_GUID_TYPE);
+    if (GetVariableStoreStatus (VariableStoreHeader) == EfiValid) {
+      if (!NeedConvertNormalToAuth) {
+        mVariableModuleGlobal->VariableGlobal.HobVariableBase = (EFI_PHYSICAL_ADDRESS) (UINTN) AllocateRuntimeCopyPool ((UINTN) VariableStoreLength, (VOID *) VariableStoreHeader);
+      } else {
+        mVariableModuleGlobal->VariableGlobal.HobVariableBase = (EFI_PHYSICAL_ADDRESS) (UINTN) ConvertNormalVarStorageToAuthVarStorage ((VOID *) VariableStoreHeader);
+      }
+      if (mVariableModuleGlobal->VariableGlobal.HobVariableBase == 0) {
+        return EFI_OUT_OF_RESOURCES;
+      }
+    } else {
+      DEBUG ((EFI_D_ERROR, "HOB Variable Store header is corrupted!\n"));
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Initializes variable store area for non-volatile and volatile variable.
 
@@ -4183,13 +4241,9 @@ VariableCommonInitialize (
 {
   EFI_STATUS                      Status;
   VARIABLE_STORE_HEADER           *VolatileVariableStore;
-  VARIABLE_STORE_HEADER           *VariableStoreHeader;
-  UINT64                          VariableStoreLength;
   UINTN                           ScratchSize;
-  EFI_HOB_GUID_TYPE               *GuidHob;
   EFI_GUID                        *VariableGuid;
   EFI_FIRMWARE_VOLUME_HEADER      *NvFvHeader;
-  BOOLEAN                         IsNormalVariableHob;
 
   //
   // Allocate runtime memory for variable driver global structure.
@@ -4231,32 +4285,11 @@ VariableCommonInitialize (
   //
   // Get HOB variable store.
   //
-  IsNormalVariableHob = FALSE;
-  GuidHob = GetFirstGuidHob (VariableGuid);
-  if (GuidHob == NULL && VariableGuid == &gEfiAuthenticatedVariableGuid) {
-    //
-    // Try getting it from normal variable HOB
-    //
-    GuidHob = GetFirstGuidHob (&gEfiVariableGuid);
-    IsNormalVariableHob = TRUE;
-  }
-  if (GuidHob != NULL) {
-    VariableStoreHeader = GET_GUID_HOB_DATA (GuidHob);
-    VariableStoreLength = GuidHob->Header.HobLength - sizeof (EFI_HOB_GUID_TYPE);
-    if (GetVariableStoreStatus (VariableStoreHeader) == EfiValid) {
-      if (!IsNormalVariableHob) {
-        mVariableModuleGlobal->VariableGlobal.HobVariableBase = (EFI_PHYSICAL_ADDRESS) (UINTN) AllocateRuntimeCopyPool ((UINTN) VariableStoreLength, (VOID *) VariableStoreHeader);
-      } else {
-        mVariableModuleGlobal->VariableGlobal.HobVariableBase = (EFI_PHYSICAL_ADDRESS) (UINTN) ConvertNormalVarStorageToAuthVarStorage ((VOID *) VariableStoreHeader);
-      }
-      if (mVariableModuleGlobal->VariableGlobal.HobVariableBase == 0) {
-        FreePool (NvFvHeader);
-        FreePool (mVariableModuleGlobal);
-        return EFI_OUT_OF_RESOURCES;
-      }
-    } else {
-      DEBUG ((EFI_D_ERROR, "HOB Variable Store header is corrupted!\n"));
-    }
+  Status = GetHobVariableStore (VariableGuid);
+  if (EFI_ERROR (Status)) {
+    FreePool (NvFvHeader);
+    FreePool (mVariableModuleGlobal);
+    return Status;
   }
 
   mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
-- 
2.7.0.windows.1

[PATCH 2/2] MdeModulePkg Variable: Make sure no more than one Variable HOB

[Star Zeng]

VariableHob may be built in PcdPeim (by PcdNvStoreDefaultValueBuffer)
or some platform module (by some tool).
The two solutions should not be co-exist.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Universal/Variable/Pei/Variable.c     | 24 ++++++++++++++++++++++
 .../Universal/Variable/RuntimeDxe/Variable.c       | 24 ++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index 1bcab3b770a5..d75a13e2b079 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -507,6 +507,30 @@ GetHobVariableStore (
 {
   EFI_HOB_GUID_TYPE              *GuidHob;
 
+  //
+  // Make sure there is no more than one Variable HOB.
+  //
+  DEBUG_CODE (
+    GuidHob = GetFirstGuidHob (&gEfiAuthenticatedVariableGuid);
+    if (GuidHob != NULL) {
+      if ((GetNextGuidHob (&gEfiAuthenticatedVariableGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+        DEBUG ((DEBUG_ERROR, "ERROR: Found two Auth Variable HOBs\n"));
+        ASSERT (FALSE);
+      } else if (GetFirstGuidHob (&gEfiVariableGuid) != NULL) {
+        DEBUG ((DEBUG_ERROR, "ERROR: Found one Auth + one Normal Variable HOBs\n"));
+        ASSERT (FALSE);
+      }
+    } else {
+      GuidHob = GetFirstGuidHob (&gEfiVariableGuid);
+      if (GuidHob != NULL) {
+        if ((GetNextGuidHob (&gEfiVariableGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+          DEBUG ((DEBUG_ERROR, "ERROR: Found two Normal Variable HOBs\n"));
+          ASSERT (FALSE);
+        }
+      }
+    }
+  );
+
   GuidHob = GetFirstGuidHob (&gEfiAuthenticatedVariableGuid);
   if (GuidHob != NULL) {
     *VariableStoreHeader = (VARIABLE_STORE_HEADER *) GET_GUID_HOB_DATA (GuidHob);
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index fca8d5380924..42b0bfda570d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -4190,6 +4190,30 @@ GetHobVariableStore (
   BOOLEAN                       NeedConvertNormalToAuth;
 
   //
+  // Make sure there is no more than one Variable HOB.
+  //
+  DEBUG_CODE (
+    GuidHob = GetFirstGuidHob (&gEfiAuthenticatedVariableGuid);
+    if (GuidHob != NULL) {
+      if ((GetNextGuidHob (&gEfiAuthenticatedVariableGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+        DEBUG ((DEBUG_ERROR, "ERROR: Found two Auth Variable HOBs\n"));
+        ASSERT (FALSE);
+      } else if (GetFirstGuidHob (&gEfiVariableGuid) != NULL) {
+        DEBUG ((DEBUG_ERROR, "ERROR: Found one Auth + one Normal Variable HOBs\n"));
+        ASSERT (FALSE);
+      }
+    } else {
+      GuidHob = GetFirstGuidHob (&gEfiVariableGuid);
+      if (GuidHob != NULL) {
+        if ((GetNextGuidHob (&gEfiVariableGuid, GET_NEXT_HOB (GuidHob)) != NULL)) {
+          DEBUG ((DEBUG_ERROR, "ERROR: Found two Normal Variable HOBs\n"));
+          ASSERT (FALSE);
+        }
+      }
+    }
+  );
+
+  //
   // Combinations supported:
   // 1. Normal NV variable store +
   //    Normal HOB variable store
-- 
2.7.0.windows.1

Re: [PATCH 0/2] Variable: Make sure no more than one Variable HOB

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star
>Zeng
>Sent: Thursday, June 28, 2018 6:22 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH 0/2] Variable: Make sure no more than one Variable
>HOB
>
>Star Zeng (2):
>  MdeModulePkg Variable: Abstract GetHobVariableStore function
>  MdeModulePkg Variable: Make sure no more than one Variable HOB
>
> MdeModulePkg/Universal/Variable/Pei/Variable.c     |  64 ++++++++---
> .../Universal/Variable/RuntimeDxe/Variable.c       | 117 +++++++++++++++---
>---
> 2 files changed, 139 insertions(+), 42 deletions(-)
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel