From 9cad030bc14e706d8986ed33f0fa8b28f0828c48 Mon Sep 17 00:00:00 2001 From: yshang1 Date: Thu, 10 Jan 2008 04:26:13 +0000 Subject: [PATCH] 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 --- .../Universal/VariablePei/Variable.c | 70 ++++++++-- .../Universal/VariablePei/Variable.h | 4 +- MdeModulePkg/Include/VariableFormat.h | 6 +- .../Universal/Variable/Pei/Variable.c | 125 ++++++++++++++++-- .../Universal/Variable/Pei/Variable.h | 4 +- .../Universal/Variable/RuntimeDxe/Variable.c | 64 +++++---- .../Universal/Variable/RuntimeDxe/Variable.h | 2 + 7 files changed, 229 insertions(+), 46 deletions(-) diff --git a/IntelFrameworkModulePkg/Universal/VariablePei/Variable.c b/IntelFrameworkModulePkg/Universal/VariablePei/Variable.c index 46c898609a..e050fbcf9d 100644 --- a/IntelFrameworkModulePkg/Universal/VariablePei/Variable.c +++ b/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 || - Variable->StartId != VARIABLE_DATA || - (sizeof (VARIABLE_HEADER) + Variable->DataSize + Variable->NameSize) > MAX_VARIABLE_SIZE - ) { + if (Variable == NULL || Variable->StartId != VARIABLE_DATA ) { 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); diff --git a/IntelFrameworkModulePkg/Universal/VariablePei/Variable.h b/IntelFrameworkModulePkg/Universal/VariablePei/Variable.h index cc82b0fd5c..aeb49bca80 100644 --- a/IntelFrameworkModulePkg/Universal/VariablePei/Variable.h +++ b/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; diff --git a/MdeModulePkg/Include/VariableFormat.h b/MdeModulePkg/Include/VariableFormat.h index c660134f0d..431f01bbb5 100644 --- a/MdeModulePkg/Include/VariableFormat.h +++ b/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 // diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c index 779a108ba1..ce79499746 100644 --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c +++ b/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 || - Variable->StartId != VARIABLE_DATA || - (sizeof (VARIABLE_HEADER) + Variable->DataSize + Variable->NameSize) > MAX_VARIABLE_SIZE - ) { + if (Variable == NULL || Variable->StartId != VARIABLE_DATA ) { 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->EndPtr = (VARIABLE_HEADER *) ((UINTN) VariableStoreHeader + VariableStoreHeader->Size); + IndexTable->StartPtr = GetStartPointer (VariableStoreHeader); + 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); diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.h b/MdeModulePkg/Universal/Variable/Pei/Variable.h index 7155cda73c..751f8bf843 100644 --- a/MdeModulePkg/Universal/Variable/Pei/Variable.h +++ b/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; diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c index 8b90bd5799..e1fec9d62c 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c +++ b/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[1] = (VARIABLE_HEADER *) (VariableStoreHeader[1] + 1); + Variable[0] = GetStartPointer (VariableStoreHeader[0]); + 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.EndPtr = (VARIABLE_HEADER *) GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase)); + Variable.StartPtr = GetStartPointer ((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)) { diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h index 2661313b9d..b756fc6454 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h +++ b/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)) -- 2.39.2