1) Fix the bug that Variable Cache Search does not be protected by lock during boot...

2) Check the integrity of Variable header. In original implementation, if not whole header is correct, then the variable will be treat as invalid. typically, if the NameSize has been programed but the DataSize not, then the variable storage would failed to set new variable.
3) Change the Variable Header Alignment from 1 to 4 bytes on x86. It avoids the DataSize or NameSize cross two blocks. For example, in original implementation, if the NameSize crosses two block, when the FLASH manipulation is interrupted after programed HSB of NameSize and prior to program LSB of NameSize on next block, then the invalid variable header will result in the Variable Storgae broken.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@4538 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 20eeebe54cc861f9a4068345394053c2864fd270..8b90bd5799f82eec3ab546513dd5edb944426751 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -172,10 +172,7 @@ Returns:
 \r
 --*/\r
 {\r
-  if (Variable == NULL ||\r
-      Variable->StartId != VARIABLE_DATA ||\r
-      (sizeof (VARIABLE_HEADER) + Variable->NameSize + Variable->DataSize) > MAX_VARIABLE_SIZE\r
-      ) {\r
+  if (Variable == NULL || Variable->StartId != VARIABLE_DATA) {\r
     return FALSE;\r
   }\r
 \r
@@ -372,6 +369,59 @@ Returns:
 }\r
 \r
 \r
+UINT32\r
+NameSizeOfVariable (\r
+  IN  VARIABLE_HEADER   *Variable\r
+  )\r
+{\r
+  //\r
+  // Check whether the header is valid fully;\r
+  // Tricky: The unprogramed data in FLASH equals 0xff.\r
+  // \r
+  if (Variable->DataSize == (UINT32) -1 || \r
+      Variable->Attributes == (UINT32) -1 || \r
+      Variable->NameSize == (UINT32) -1) {\r
+    return 0;\r
+  }\r
+  return Variable->NameSize;\r
+}\r
+\r
+UINT32\r
+DataSizeOfVariable (\r
+  IN  VARIABLE_HEADER   *Variable\r
+  )\r
+{\r
+  //\r
+  // Check whether the header is valid fully;\r
+  // Tricky: The unprogramed data in FLASH equals 0xff.\r
+  // \r
+  if (Variable->DataSize == (UINT32) -1 || \r
+      Variable->Attributes == (UINT32) -1 || \r
+      Variable->NameSize == (UINT32) -1) {\r
+    return 0;\r
+  }\r
+  return Variable->DataSize;\r
+}\r
+\r
+UINT32\r
+AttributesOfVariable (\r
+  IN  VARIABLE_HEADER   *Variable\r
+  )\r
+{\r
+\r
+  //\r
+  // Check whether the header is valid fully;\r
+  // Tricky: The unprogramed data in FLASH equals 0xff.\r
+  // \r
+  if (Variable->DataSize == (UINT32) -1 || \r
+      Variable->Attributes == (UINT32) -1 || \r
+      Variable->NameSize == (UINT32) -1) {\r
+    return 0;\r
+  }\r
+  return Variable->Attributes;\r
+}\r
+\r
+\r
 UINT8 *\r
 GetVariableDataPtr (\r
   IN  VARIABLE_HEADER   *Variable\r
@@ -395,7 +445,7 @@ Returns:
   //\r
   // Be careful about pad size for alignment\r
   //\r
-  return (UINT8 *) ((UINTN) GET_VARIABLE_NAME_PTR (Variable) + Variable->NameSize + GET_PAD_SIZE (Variable->NameSize));\r
+  return (UINT8 *) ((UINTN) GET_VARIABLE_NAME_PTR (Variable) + NameSizeOfVariable (Variable) + GET_PAD_SIZE (NameSizeOfVariable (Variable)));\r
 }\r
 \r
 \r
@@ -425,7 +475,7 @@ Returns:
   //\r
   // Be careful about pad size for alignment\r
   //\r
-  return (VARIABLE_HEADER *) ((UINTN) GetVariableDataPtr (Variable) + Variable->DataSize + GET_PAD_SIZE (Variable->DataSize));\r
+  return (VARIABLE_HEADER *) ((UINTN) GetVariableDataPtr (Variable) + DataSizeOfVariable (Variable) + GET_PAD_SIZE (DataSizeOfVariable (Variable)));\r
 }\r
 \r
 \r
@@ -729,13 +779,6 @@ Returns:
   VARIABLE_STORE_HEADER *VariableStoreHeader[2];\r
   UINTN                 Index;\r
 \r
-  //\r
-  // We aquire the lock at the entry of FindVariable as GetVariable, GetNextVariableName\r
-  // SetVariable all call FindVariable at entry point. Please move "Aquire Lock" to\r
-  // the correct places if this assumption does not hold TRUE anymore.\r
-  //\r
-  AcquireLockOnlyAtBootTime(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);\r
-\r
   //\r
   // 0: Volatile, 1: Non-Volatile\r
   // The index and attributes mapping must be kept in this order as RuntimeServiceGetNextVariableName\r
@@ -770,7 +813,8 @@ Returns:
             return EFI_SUCCESS;\r
           } else {\r
             if (CompareGuid (VendorGuid, &Variable[Index]->VendorGuid)) {\r
-              if (!CompareMem (VariableName, GET_VARIABLE_NAME_PTR (Variable[Index]), Variable[Index]->NameSize)) {\r
+              ASSERT (NameSizeOfVariable (Variable[Index]) != 0);\r
+              if (!CompareMem (VariableName, GET_VARIABLE_NAME_PTR (Variable[Index]), NameSizeOfVariable (Variable[Index]))) {\r
                 PtrTrack->CurrPtr   = Variable[Index];\r
                 PtrTrack->Volatile  = (BOOLEAN)(Index == 0);\r
                 return EFI_SUCCESS;\r
@@ -833,6 +877,8 @@ RuntimeServiceGetVariable (
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
+  AcquireLockOnlyAtBootTime(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);\r
+\r
   //\r
   // Find existing variable\r
   //\r
@@ -840,7 +886,7 @@ RuntimeServiceGetVariable (
   if ((Status == EFI_BUFFER_TOO_SMALL) || (Status == EFI_SUCCESS)){\r
     // Hit in the Cache\r
     UpdateVariableInfo (VariableName, VendorGuid, FALSE, TRUE, FALSE, FALSE, TRUE);\r
-    return Status;\r
+    goto Done;\r
   }\r
   \r
   Status = FindVariable (VariableName, VendorGuid, &Variable, &mVariableModuleGlobal->VariableGlobal);\r
@@ -851,7 +897,9 @@ RuntimeServiceGetVariable (
   //\r
   // Get data size\r
   //\r
-  VarDataSize = Variable.CurrPtr->DataSize;\r
+  VarDataSize = DataSizeOfVariable (Variable.CurrPtr);\r
+  ASSERT (VarDataSize != 0);\r
+\r
   if (*DataSize >= VarDataSize) {\r
     if (Data == NULL) {\r
       Status = EFI_INVALID_PARAMETER;\r
@@ -917,6 +965,8 @@ RuntimeServiceGetNextVariableName (
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
+  AcquireLockOnlyAtBootTime(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);\r
+\r
   Status = FindVariable (VariableName, VendorGuid, &Variable, &mVariableModuleGlobal->VariableGlobal);\r
   if (Variable.CurrPtr == NULL || EFI_ERROR (Status)) {\r
     goto Done;\r
@@ -954,7 +1004,9 @@ RuntimeServiceGetNextVariableName (
     //\r
     if (IsValidVariableHeader (Variable.CurrPtr) && Variable.CurrPtr->State == VAR_ADDED) {\r
       if (!(EfiAtRuntime () && !(Variable.CurrPtr->Attributes & EFI_VARIABLE_RUNTIME_ACCESS))) {\r
-        VarNameSize = Variable.CurrPtr->NameSize;\r
+        VarNameSize = NameSizeOfVariable (Variable.CurrPtr);\r
+        ASSERT (VarNameSize != 0);\r
+\r
         if (VarNameSize <= *VariableNameSize) {\r
           CopyMem (\r
             VariableName,\r
@@ -1037,11 +1089,7 @@ RuntimeServiceSetVariable (
   UINTN                   *NonVolatileOffset;\r
   UINT32                  Instance;\r
   BOOLEAN                 Volatile;\r
-\r
-  Reclaimed         = FALSE;\r
-  VolatileOffset    = &mVariableModuleGlobal->VolatileLastVariableOffset;\r
-  NonVolatileOffset = &mVariableModuleGlobal->NonVolatileLastVariableOffset;\r
-  Instance          = mVariableModuleGlobal->FvbInstance;\r
+  EFI_PHYSICAL_ADDRESS    Point;\r
 \r
   //\r
   // Check input parameters\r
@@ -1055,6 +1103,7 @@ RuntimeServiceSetVariable (
   if ((Attributes & (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)) == EFI_VARIABLE_RUNTIME_ACCESS) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
+\r
   //\r
   //  The size of the VariableName, including the Unicode Null in bytes plus\r
   //  the DataSize is limited to maximum size of MAX_HARDWARE_ERROR_VARIABLE_SIZE (32K)\r
@@ -1075,6 +1124,36 @@ RuntimeServiceSetVariable (
       return EFI_INVALID_PARAMETER;\r
     }  \r
   }  \r
+\r
+  AcquireLockOnlyAtBootTime(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);\r
+\r
+  Reclaimed         = FALSE;\r
+  Instance          = mVariableModuleGlobal->FvbInstance;\r
+  VolatileOffset    = &mVariableModuleGlobal->VolatileLastVariableOffset;\r
+\r
+  //\r
+  // Consider reentrant in MCA/INIT/NMI. It needs be reupdated;\r
+  //\r
+  if (1 < InterlockedIncrement (&mVariableModuleGlobal->VariableGlobal.ReentrantState)) {\r
+    {\r
+      volatile int tt = 1;\r
+      while (tt) {\r
+      }\r
+    }\r
+    Point = mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;;\r
+    //\r
+    // Parse non-volatile variable data and get last variable offset\r
+    //\r
+    NextVariable  = (VARIABLE_HEADER *) (UINTN) (Point + sizeof (VARIABLE_STORE_HEADER));\r
+    while (IsValidVariableHeader (NextVariable)) {\r
+      NextVariable = GetNextVariablePtr (NextVariable);\r
+    }\r
+    mVariableModuleGlobal->NonVolatileLastVariableOffset = (UINTN) NextVariable - (UINTN) Point;\r
+  }\r
+\r
+  NonVolatileOffset = &mVariableModuleGlobal->NonVolatileLastVariableOffset;\r
+  \r
+\r
   //\r
   // Check whether the input variable is already existed\r
   //\r
@@ -1131,7 +1210,7 @@ RuntimeServiceSetVariable (
     // If the variable is marked valid and the same data has been passed in\r
     // then return to the caller immediately.\r
     //\r
-    if (Variable.CurrPtr->DataSize == DataSize &&\r
+    if (DataSizeOfVariable (Variable.CurrPtr) == DataSize &&\r
         (CompareMem (Data, GetVariableDataPtr (Variable.CurrPtr), DataSize) == 0)) {\r
       \r
       UpdateVariableInfo (VariableName, VendorGuid, Volatile, FALSE, TRUE, FALSE, FALSE);\r
@@ -1396,7 +1475,9 @@ RuntimeServiceSetVariable (
   UpdateVariableCache (VariableName, VendorGuid, Attributes, DataSize, Data);\r
 \r
 Done:\r
+  InterlockedDecrement (&mVariableModuleGlobal->VariableGlobal.ReentrantState);\r
   ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);\r
+\r
   return Status;\r
 }\r
 \r
@@ -1586,6 +1667,7 @@ Returns:
 \r
 \r
   EfiInitializeLock(&mVariableModuleGlobal->VariableGlobal.VariableServicesLock, TPL_NOTIFY);\r
+  mVariableModuleGlobal->VariableGlobal.ReentrantState = 0;\r
 \r
   //\r
   // Allocate memory for volatile variable store\r

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index b2af852310fca4a4d0c470d1ea4975b9a7fc6f0c..2661313b9d8a026b32fe9114c82d234216e675ff 100644 (file)
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -71,6 +71,7 @@ typedef struct {
   EFI_PHYSICAL_ADDRESS  VolatileVariableBase;\r
   EFI_PHYSICAL_ADDRESS  NonVolatileVariableBase;\r
   EFI_LOCK              VariableServicesLock;\r
+  UINT32                ReentrantState;\r
 } VARIABLE_GLOBAL;\r
 \r
 typedef struct {\r