Align the header of variable from 1 to 4, which can avoid the size of variable content corrupt if the critical data cross two flash block.

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

IntelFrameworkModulePkg/Universal/VariablePei/Variable.c

@@ -80,6 +80,59 @@ Returns:
 
 }
 
+
+UINT32
+NameSizeOfVariable (
+  IN  VARIABLE_HEADER   *Variable
+  )
+{
+  //
+  // Check whether the header is valid fully;
+  // Tricky: The unprogramed data in FLASH equals 0xff.
+  // 
+  if (Variable->DataSize == (UINT32) -1 || 
+      Variable->Attributes == (UINT32) -1 || 
+      Variable->NameSize == (UINT32) -1) {
+    return 0;
+  }
+  return Variable->NameSize;
+}
+
+UINT32
+DataSizeOfVariable (
+  IN  VARIABLE_HEADER   *Variable
+  )
+{
+  //
+  // Check whether the header is valid fully;
+  // Tricky: The unprogramed data in FLASH equals 0xff.
+  // 
+  if (Variable->DataSize == (UINT32) -1 || 
+      Variable->Attributes == (UINT32) -1 || 
+      Variable->NameSize == (UINT32) -1) {
+    return 0;
+  }
+  return Variable->DataSize;
+}
+
+UINT32
+AttributesOfVariable (
+  IN  VARIABLE_HEADER   *Variable
+  )
+{
+
+  //
+  // Check whether the header is valid fully;
+  // Tricky: The unprogramed data in FLASH equals 0xff.
+  // 
+  if (Variable->DataSize == (UINT32) -1 || 
+      Variable->Attributes == (UINT32) -1 || 
+      Variable->NameSize == (UINT32) -1) {
+    return 0;
+  }
+  return Variable->Attributes;
+}
+
 STATIC
 VARIABLE_HEADER *
 GetNextVariablePtr (
@@ -100,7 +153,7 @@ Returns:
 
 --*/
 {
-  return (VARIABLE_HEADER *) ((UINTN) GET_VARIABLE_DATA_PTR (Variable) + Variable->DataSize + GET_PAD_SIZE (Variable->DataSize));
+  return (VARIABLE_HEADER *) HEADER_ALIGN ((UINTN) GET_VARIABLE_DATA_PTR (Variable) + DataSizeOfVariable (Variable) + GET_PAD_SIZE (DataSizeOfVariable (Variable)));
 }
 
 STATIC
@@ -124,10 +177,7 @@ Returns:
 
 --*/
 {
-  if (Variable == NULL ||
+  if (Variable == NULL || Variable->StartId != VARIABLE_DATA ) {
-      Variable->StartId != VARIABLE_DATA ||
-      (sizeof (VARIABLE_HEADER) + Variable->DataSize + Variable->NameSize) > MAX_VARIABLE_SIZE
-      ) {
     return FALSE;
   }
 
@@ -221,7 +271,8 @@ Returns:
         (((INT32 *) VendorGuid)[2] == ((INT32 *) &Variable->VendorGuid)[2]) &&
         (((INT32 *) VendorGuid)[3] == ((INT32 *) &Variable->VendorGuid)[3])
         ) {
-      if (!CompareMem (VariableName, GET_VARIABLE_NAME_PTR (Variable), Variable->NameSize)) {
+      ASSERT (NameSizeOfVariable (Variable) != 0);
+      if (!CompareMem (VariableName, GET_VARIABLE_NAME_PTR (Variable), NameSizeOfVariable (Variable))) {
         PtrTrack->CurrPtr = Variable;
         return EFI_SUCCESS;
       }
@@ -290,7 +341,6 @@ Returns:
     for (Count = 0; Count < IndexTable->Length; Count++)
     {
       MaxIndex = GetVariableByIndex (IndexTable, Count);
-
       if (CompareWithValidVariable (MaxIndex, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
         PtrTrack->StartPtr  = IndexTable->StartPtr;
         PtrTrack->EndPtr    = IndexTable->EndPtr;
@@ -429,7 +479,7 @@ Returns:
   //
   // Get data size
   //
-  VarDataSize = Variable.CurrPtr->DataSize;
+  VarDataSize = DataSizeOfVariable (Variable.CurrPtr);
   if (*DataSize >= VarDataSize) {
     (*PeiServices)->CopyMem (Data, GET_VARIABLE_DATA_PTR (Variable.CurrPtr), VarDataSize);
 
@@ -555,7 +605,9 @@ Returns:
   while (!(Variable.CurrPtr >= Variable.EndPtr || Variable.CurrPtr == NULL)) {
     if (IsValidVariableHeader (Variable.CurrPtr)) {
       if (Variable.CurrPtr->State == VAR_ADDED) {
-        VarNameSize = (UINTN) Variable.CurrPtr->NameSize;
+        ASSERT (NameSizeOfVariable (Variable.CurrPtr) != 0);
+
+        VarNameSize = (UINTN) NameSizeOfVariable (Variable.CurrPtr);
         if (VarNameSize <= *VariableNameSize) {
           (*PeiServices)->CopyMem (VariableName, GET_VARIABLE_NAME_PTR (Variable.CurrPtr), VarNameSize);
 

IntelFrameworkModulePkg/Universal/VariablePei/Variable.h

@@ -42,10 +42,12 @@ Abstract:
 #define GET_PAD_SIZE(a) (((~a) + 1) & (ALIGNMENT - 1))
 #endif
 
+#define HEADER_ALIGN(Header)  (((UINTN) (Header) + HEADER_ALIGNMENT - 1) & (~(HEADER_ALIGNMENT - 1)))
+
 #define GET_VARIABLE_NAME_PTR(a)  (CHAR16 *) ((UINTN) (a) + sizeof (VARIABLE_HEADER))
 
 #define GET_VARIABLE_DATA_PTR(a) \
-  (UINT8 *) ((UINTN) GET_VARIABLE_NAME_PTR (a) + (a)->NameSize + GET_PAD_SIZE ((a)->NameSize))
+  (UINT8 *) ((UINTN) GET_VARIABLE_NAME_PTR (a) + NameSizeOfVariable(a) + GET_PAD_SIZE (NameSizeOfVariable(a) ))
 
 typedef struct {
   VARIABLE_HEADER *CurrPtr;

MdeModulePkg/Include/VariableFormat.h

@@ -38,11 +38,13 @@
 // 8 is for IPF archtecture.
 //
 #if defined (MDE_CPU_IPF)
-#define ALIGNMENT  8
+#define ALIGNMENT         8
 #else
-#define ALIGNMENT  1
+#define ALIGNMENT         1
 #endif
 
+#define HEADER_ALIGNMENT  4
+
 //
 // Variable Store Status
 //

MdeModulePkg/Universal/Variable/Pei/Variable.c

@@ -68,6 +68,111 @@ Returns:
 
 }
 
+VARIABLE_HEADER *
+GetStartPointer (
+  IN VARIABLE_STORE_HEADER       *VarStoreHeader
+  )
+/*++
+
+Routine Description:
+
+  This code gets the pointer to the first variable memory pointer byte
+
+Arguments:
+
+  VarStoreHeader        Pointer to the Variable Store Header.
+
+Returns:
+
+  VARIABLE_HEADER*      Pointer to last unavailable Variable Header
+
+--*/
+{
+  //
+  // The end of variable store
+  //
+  return (VARIABLE_HEADER *) HEADER_ALIGN (VarStoreHeader + 1);
+}
+
+VARIABLE_HEADER *
+GetEndPointer (
+  IN VARIABLE_STORE_HEADER       *VarStoreHeader
+  )
+/*++
+
+Routine Description:
+
+  This code gets the pointer to the last variable memory pointer byte
+
+Arguments:
+
+  VarStoreHeader        Pointer to the Variable Store Header.
+
+Returns:
+
+  VARIABLE_HEADER*      Pointer to last unavailable Variable Header
+
+--*/
+{
+  //
+  // The end of variable store
+  //
+  return (VARIABLE_HEADER *) HEADER_ALIGN ((UINTN) VarStoreHeader + VarStoreHeader->Size);
+}
+
+UINT32
+NameSizeOfVariable (
+  IN  VARIABLE_HEADER   *Variable
+  )
+{
+  //
+  // Check whether the header is valid fully;
+  // Tricky: The unprogramed data in FLASH equals 0xff.
+  // 
+  if (Variable->DataSize == (UINT32) -1 || 
+      Variable->Attributes == (UINT32) -1 || 
+      Variable->NameSize == (UINT32) -1) {
+    return 0;
+  }
+  return Variable->NameSize;
+}
+
+UINT32
+DataSizeOfVariable (
+  IN  VARIABLE_HEADER   *Variable
+  )
+{
+  //
+  // Check whether the header is valid fully;
+  // Tricky: The unprogramed data in FLASH equals 0xff.
+  // 
+  if (Variable->DataSize == (UINT32) -1 || 
+      Variable->Attributes == (UINT32) -1 || 
+      Variable->NameSize == (UINT32) -1) {
+    return 0;
+  }
+  return Variable->DataSize;
+}
+
+UINT32
+AttributesOfVariable (
+  IN  VARIABLE_HEADER   *Variable
+  )
+{
+
+  //
+  // Check whether the header is valid fully;
+  // Tricky: The unprogramed data in FLASH equals 0xff.
+  // 
+  if (Variable->DataSize == (UINT32) -1 || 
+      Variable->Attributes == (UINT32) -1 || 
+      Variable->NameSize == (UINT32) -1) {
+    return 0;
+  }
+  return Variable->Attributes;
+}
+
+
 STATIC
 VARIABLE_HEADER *
 GetNextVariablePtr (
@@ -88,7 +193,7 @@ Returns:
 
 --*/
 {
-  return (VARIABLE_HEADER *) ((UINTN) GET_VARIABLE_DATA_PTR (Variable) + Variable->DataSize + GET_PAD_SIZE (Variable->DataSize));
+  return (VARIABLE_HEADER *) HEADER_ALIGN ((UINTN) GET_VARIABLE_DATA_PTR (Variable) + DataSizeOfVariable (Variable) + GET_PAD_SIZE (DataSizeOfVariable (Variable)));
 }
 
 STATIC
@@ -112,10 +217,7 @@ Returns:
 
 --*/
 {
-  if (Variable == NULL ||
+  if (Variable == NULL || Variable->StartId != VARIABLE_DATA ) {
-      Variable->StartId != VARIABLE_DATA ||
-      (sizeof (VARIABLE_HEADER) + Variable->DataSize + Variable->NameSize) > MAX_VARIABLE_SIZE
-      ) {
     return FALSE;
   }
 
@@ -209,7 +311,8 @@ Returns:
         (((INT32 *) VendorGuid)[2] == ((INT32 *) &Variable->VendorGuid)[2]) &&
         (((INT32 *) VendorGuid)[3] == ((INT32 *) &Variable->VendorGuid)[3])
         ) {
-      if (!CompareMem (VariableName, GET_VARIABLE_NAME_PTR (Variable), Variable->NameSize)) {
+      ASSERT (NameSizeOfVariable (Variable) != 0);
+      if (!CompareMem (VariableName, GET_VARIABLE_NAME_PTR (Variable), NameSizeOfVariable (Variable))) {
         PtrTrack->CurrPtr = Variable;
         return EFI_SUCCESS;
       }
@@ -314,8 +417,8 @@ Returns:
       //
       // Find the variable by walk through non-volatile variable store
       //
-      IndexTable->StartPtr  = (VARIABLE_HEADER *) (VariableStoreHeader + 1);
+      IndexTable->StartPtr  = GetStartPointer (VariableStoreHeader);
-      IndexTable->EndPtr    = (VARIABLE_HEADER *) ((UINTN) VariableStoreHeader + VariableStoreHeader->Size);
+      IndexTable->EndPtr    = GetEndPointer (VariableStoreHeader);
 
       //
       // Start Pointers for the variable.
@@ -418,7 +521,7 @@ Returns:
   //
   // Get data size
   //
-  VarDataSize = Variable.CurrPtr->DataSize;
+  VarDataSize = DataSizeOfVariable (Variable.CurrPtr);
   if (*DataSize >= VarDataSize) {
     (*PeiServices)->CopyMem (Data, GET_VARIABLE_DATA_PTR (Variable.CurrPtr), VarDataSize);
 
@@ -494,7 +597,9 @@ Returns:
   while (!(Variable.CurrPtr >= Variable.EndPtr || Variable.CurrPtr == NULL)) {
     if (IsValidVariableHeader (Variable.CurrPtr)) {
       if (Variable.CurrPtr->State == VAR_ADDED) {
-        VarNameSize = (UINTN) Variable.CurrPtr->NameSize;
+        ASSERT (NameSizeOfVariable (Variable.CurrPtr) != 0);
+
+        VarNameSize = (UINTN) NameSizeOfVariable (Variable.CurrPtr);
         if (VarNameSize <= *VariableNameSize) {
           (*PeiServices)->CopyMem (VariableName, GET_VARIABLE_NAME_PTR (Variable.CurrPtr), VarNameSize);
 

MdeModulePkg/Universal/Variable/Pei/Variable.h

@@ -41,10 +41,12 @@ Abstract:
 #define GET_PAD_SIZE(a) (((~a) + 1) & (ALIGNMENT - 1))
 #endif
 
+#define HEADER_ALIGN(Header)  (((UINTN) (Header) + HEADER_ALIGNMENT - 1) & (~(HEADER_ALIGNMENT - 1)))
+
 #define GET_VARIABLE_NAME_PTR(a)  (CHAR16 *) ((UINTN) (a) + sizeof (VARIABLE_HEADER))
 
 #define GET_VARIABLE_DATA_PTR(a) \
-  (UINT8 *) ((UINTN) GET_VARIABLE_NAME_PTR (a) + (a)->NameSize + GET_PAD_SIZE ((a)->NameSize))
+  (UINT8 *) ((UINTN) GET_VARIABLE_NAME_PTR (a) + NameSizeOfVariable(a) + GET_PAD_SIZE (NameSizeOfVariable(a)))
 
 typedef struct {
   VARIABLE_HEADER *CurrPtr;

MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c

@@ -152,7 +152,6 @@ UpdateVariableInfo (
 }
 
 
-
 BOOLEAN
 IsValidVariableHeader (
   IN  VARIABLE_HEADER   *Variable
@@ -475,9 +474,34 @@ Returns:
   //
   // Be careful about pad size for alignment
   //
-  return (VARIABLE_HEADER *) ((UINTN) GetVariableDataPtr (Variable) + DataSizeOfVariable (Variable) + GET_PAD_SIZE (DataSizeOfVariable (Variable)));
+  return (VARIABLE_HEADER *) HEADER_ALIGN (((UINTN) GetVariableDataPtr (Variable) + DataSizeOfVariable (Variable) + GET_PAD_SIZE (DataSizeOfVariable (Variable))));
 }
 
+VARIABLE_HEADER *
+GetStartPointer (
+  IN VARIABLE_STORE_HEADER       *VarStoreHeader
+  )
+/*++
+
+Routine Description:
+
+  This code gets the pointer to the first variable memory pointer byte
+
+Arguments:
+
+  VarStoreHeader        Pointer to the Variable Store Header.
+
+Returns:
+
+  VARIABLE_HEADER*      Pointer to last unavailable Variable Header
+
+--*/
+{
+  //
+  // The end of variable store
+  //
+  return (VARIABLE_HEADER *) HEADER_ALIGN (VarStoreHeader + 1);
+}
 
 VARIABLE_HEADER *
 GetEndPointer (
@@ -502,7 +526,7 @@ Returns:
   //
   // The end of variable store
   //
-  return (VARIABLE_HEADER *) ((UINTN) VarStoreHeader + VarStoreHeader->Size);
+  return (VARIABLE_HEADER *) HEADER_ALIGN ((UINTN) VarStoreHeader + VarStoreHeader->Size);
 }
 
 
@@ -545,8 +569,7 @@ Returns:
   //
   // Start Pointers for the variable.
   //
-  Variable        = (VARIABLE_HEADER *) (VariableStoreHeader + 1);
+  Variable        = GetStartPointer (VariableStoreHeader);
-
   ValidBufferSize = sizeof (VARIABLE_STORE_HEADER);
 
   while (IsValidVariableHeader (Variable)) {
@@ -572,12 +595,12 @@ Returns:
   // Copy variable store header
   //
   CopyMem (CurrPtr, VariableStoreHeader, sizeof (VARIABLE_STORE_HEADER));
-  CurrPtr += sizeof (VARIABLE_STORE_HEADER);
+  CurrPtr = (UINT8 *) GetStartPointer (VariableStoreHeader);
 
   //
   // Start Pointers for the variable.
   //
-  Variable = (VARIABLE_HEADER *) (VariableStoreHeader + 1);
+  Variable = GetStartPointer (VariableStoreHeader);
 
   while (IsValidVariableHeader (Variable)) {
     NextVariable = GetNextVariablePtr (Variable);
@@ -791,8 +814,8 @@ Returns:
   // Start Pointers for the variable.
   // Actual Data Pointer where data can be written.
   //
-  Variable[0] = (VARIABLE_HEADER *) (VariableStoreHeader[0] + 1);
+  Variable[0] = GetStartPointer (VariableStoreHeader[0]);
-  Variable[1] = (VARIABLE_HEADER *) (VariableStoreHeader[1] + 1);
+  Variable[1] = GetStartPointer (VariableStoreHeader[1]);
 
   if (VariableName[0] != 0 && VendorGuid == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -801,7 +824,7 @@ Returns:
   // Find the variable by walk through volatile and then non-volatile variable store
   //
   for (Index = 0; Index < 2; Index++) {
-    PtrTrack->StartPtr  = (VARIABLE_HEADER *) (VariableStoreHeader[Index] + 1);
+    PtrTrack->StartPtr  = GetStartPointer (VariableStoreHeader[Index]);
     PtrTrack->EndPtr    = GetEndPointer (VariableStoreHeader[Index]);
 
     while (IsValidVariableHeader (Variable[Index]) && (Variable[Index] <= GetEndPointer (VariableStoreHeader[Index]))) {
@@ -987,8 +1010,8 @@ RuntimeServiceGetNextVariableName (
     if (Variable.CurrPtr >= Variable.EndPtr || Variable.CurrPtr == NULL) {
       Variable.Volatile = (BOOLEAN) (Variable.Volatile ^ ((BOOLEAN) 0x1));
       if (!Variable.Volatile) {
-        Variable.StartPtr = (VARIABLE_HEADER *) ((UINTN) (mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase + sizeof (VARIABLE_STORE_HEADER)));
+        Variable.StartPtr = GetStartPointer ((VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase);
-        Variable.EndPtr = (VARIABLE_HEADER *) GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase));
+        Variable.EndPtr   = GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase));
       } else {
         Status = EFI_NOT_FOUND;
         goto Done;
@@ -1135,16 +1158,11 @@ RuntimeServiceSetVariable (
   // Consider reentrant in MCA/INIT/NMI. It needs be reupdated;
   //
   if (1 < InterlockedIncrement (&mVariableModuleGlobal->VariableGlobal.ReentrantState)) {
-    {
-      volatile int tt = 1;
-      while (tt) {
-      }
-    }
     Point = mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;;
     //
     // Parse non-volatile variable data and get last variable offset
     //
-    NextVariable  = (VARIABLE_HEADER *) (UINTN) (Point + sizeof (VARIABLE_STORE_HEADER));
+    NextVariable  = GetStartPointer ((VARIABLE_STORE_HEADER *) (UINTN) Point);
     while (IsValidVariableHeader (NextVariable)) {
       NextVariable = GetNextVariablePtr (NextVariable);
     }
@@ -1399,7 +1417,7 @@ RuntimeServiceSetVariable (
       goto Done;
     }
 
-    *NonVolatileOffset = *NonVolatileOffset + VarSize;
+    *NonVolatileOffset = HEADER_ALIGN (*NonVolatileOffset + VarSize);
 
   } else {
     //
@@ -1444,7 +1462,7 @@ RuntimeServiceSetVariable (
       goto Done;
     }
 
-    *VolatileOffset = *VolatileOffset + VarSize;
+    *VolatileOffset = HEADER_ALIGN (*VolatileOffset + VarSize);
   }
   //
   // Mark the old variable as deleted
@@ -1580,7 +1598,7 @@ RuntimeServiceQueryVariableInfo (
   //
   // Point to the starting address of the variables.
   //
-  Variable = (VARIABLE_HEADER *) (VariableStoreHeader + 1);
+  Variable = GetStartPointer (VariableStoreHeader);
 
   //
   // Now walk through the related variable store.
@@ -1684,7 +1702,7 @@ Returns:
   //  Variable Specific Data
   //
   mVariableModuleGlobal->VariableGlobal.VolatileVariableBase = (EFI_PHYSICAL_ADDRESS) (UINTN) VolatileVariableStore;
-  mVariableModuleGlobal->VolatileLastVariableOffset = sizeof (VARIABLE_STORE_HEADER);
+  mVariableModuleGlobal->VolatileLastVariableOffset = (UINTN) GetStartPointer (VolatileVariableStore) - (UINTN) VolatileVariableStore;
 
   VolatileVariableStore->Signature                  = VARIABLE_STORE_SIGNATURE;
   VolatileVariableStore->Size                       = VARIABLE_STORE_SIZE;
@@ -1780,7 +1798,7 @@ Returns:
     //
     // Parse non-volatile variable data and get last variable offset
     //
-    NextVariable  = (VARIABLE_HEADER *) (CurrPtr + sizeof (VARIABLE_STORE_HEADER));
+    NextVariable  = GetStartPointer ((VARIABLE_STORE_HEADER *) CurrPtr);
     Status        = EFI_SUCCESS;
 
     while (IsValidVariableHeader (NextVariable)) {

MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h

@@ -57,6 +57,8 @@ Abstract:
 #define GET_PAD_SIZE(a) (((~a) + 1) & (ALIGNMENT - 1))
 #endif
 
+#define HEADER_ALIGN(Header)  (((UINTN) (Header) + HEADER_ALIGNMENT - 1) & (~(HEADER_ALIGNMENT - 1)))
+
 #define GET_VARIABLE_NAME_PTR(a)  (CHAR16 *) ((UINTN) (a) + sizeof (VARIABLE_HEADER))