SecurityPkg Variable: Remove mStorageData buffer allocation and use Scratch buffer...

It can reduce MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)) size of SMRAM consumption.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Guo Dong <guo.dong@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@14855 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 90a5d51a9e3aaf5ec40aaf05e093cd769f826272..ee45ec97767de0e9b45b730f5e6cec97068170e1 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -48,14 +48,6 @@ CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 //\r
 VOID  *mHashCtx = NULL;\r
 \r
-//\r
-// Pointer to runtime buffer.\r
-// For "Append" operation to an existing variable, a read/modify/write operation\r
-// is supported by firmware internally. Reserve runtime buffer to cache previous\r
-// variable data in runtime phase because memory allocation is forbidden in virtual mode.\r
-//\r
-VOID  *mStorageArea = NULL;\r
-\r
 //\r
 // The serialization of the values of the VariableName, VendorGuid and Attributes\r
 // parameters of the SetVariable() call and the TimeStamp component of the\r
@@ -191,14 +183,6 @@ AutenticatedVariableServiceInitialize (
     return EFI_OUT_OF_RESOURCES;\r
   }\r
 \r
-  //\r
-  // Reserved runtime buffer for "Append" operation in virtual mode.\r
-  //\r
-  mStorageArea  = AllocateRuntimePool (MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)));\r
-  if (mStorageArea == NULL) {\r
-    return EFI_OUT_OF_RESOURCES;\r
-  }\r
-\r
   //\r
   // Prepare runtime buffer for serialized data of time-based authenticated\r
   // Variable, i.e. (VariableName, VendorGuid, Attributes, TimeStamp, Data).\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
index 1fd687b03364c87b1bf063223d09f65780807700..745e3c7d8fbd9f640e73c9b81492d52ab34681df 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
@@ -329,7 +329,6 @@ VerifyTimeBasedPayload (
 extern UINT8  mPubKeyStore[MAX_KEYDB_SIZE];\r
 extern UINT32 mPubKeyNumber;\r
 extern VOID   *mHashCtx;\r
-extern VOID   *mStorageArea;\r
 extern UINT8  *mSerializationRuntimeBuffer;\r
 \r
 #endif\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index adcf25c313ae391836662e5996af1b710ca17755..28d026a0a8eb1a8d9b64827bc47692b0e55623f6 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -1752,7 +1752,9 @@ UpdateVariable (
   VARIABLE_POINTER_TRACK              NvVariable;\r
   VARIABLE_STORE_HEADER               *VariableStoreHeader;\r
   UINTN                               CacheOffset;\r
-  UINTN                               BufSize;\r
+  UINT8                               *BufferForMerge;\r
+  UINTN                               MergedBufSize;\r
+  BOOLEAN                             DataReady;\r
   UINTN                               DataOffset;\r
 \r
   if (mVariableModuleGlobal->FvbInstance == NULL) {\r
@@ -1802,7 +1804,8 @@ UpdateVariable (
   //\r
   NextVariable = GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.VolatileVariableBase));\r
   ScratchSize = MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize));\r
-\r
+  SetMem (NextVariable, ScratchSize, 0xff);\r
+  DataReady = FALSE;\r
 \r
   if (Variable->CurrPtr != NULL) {\r
     //\r
@@ -1894,7 +1897,7 @@ UpdateVariable (
     // then return to the caller immediately.\r
     //\r
     if (DataSizeOfVariable (Variable->CurrPtr) == DataSize &&\r
-        (CompareMem (Data, GetVariableDataPtr (Variable->CurrPtr), DataSize) == 0)  &&\r
+        (CompareMem (Data, GetVariableDataPtr (Variable->CurrPtr), DataSize) == 0) &&\r
         ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0) &&\r
         (TimeStamp == NULL)) {\r
       //\r
@@ -1911,42 +1914,42 @@ UpdateVariable (
       //\r
       if ((Attributes & EFI_VARIABLE_APPEND_WRITE) != 0) {\r
         //\r
-        // Cache the previous variable data into StorageArea.\r
+        // NOTE: From 0 to DataOffset of NextVariable is reserved for Variable Header and Name.\r
+        // From DataOffset of NextVariable is to save the existing variable data.\r
         //\r
         DataOffset = sizeof (VARIABLE_HEADER) + Variable->CurrPtr->NameSize + GET_PAD_SIZE (Variable->CurrPtr->NameSize);\r
-        CopyMem (mStorageArea, (UINT8*)((UINTN) Variable->CurrPtr + DataOffset), Variable->CurrPtr->DataSize);\r
+        BufferForMerge = (UINT8 *) ((UINTN) NextVariable + DataOffset);\r
+        CopyMem (BufferForMerge, (UINT8 *) ((UINTN) Variable->CurrPtr + DataOffset), Variable->CurrPtr->DataSize);\r
 \r
         //\r
         // Set Max Common Variable Data Size as default MaxDataSize \r
         //\r
-        MaxDataSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName));\r
-\r
+        MaxDataSize = PcdGet32 (PcdMaxVariableSize) - DataOffset;\r
 \r
         if ((CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&\r
             ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) ||\r
             (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid) && (StrCmp (VariableName, EFI_KEY_EXCHANGE_KEY_NAME) == 0))) {\r
-\r
           //\r
           // For variables with formatted as EFI_SIGNATURE_LIST, the driver shall not perform an append of\r
           // EFI_SIGNATURE_DATA values that are already part of the existing variable value.\r
           //\r
           Status = AppendSignatureList (\r
-                     mStorageArea, \r
+                     BufferForMerge,\r
                      Variable->CurrPtr->DataSize, \r
                      MaxDataSize - Variable->CurrPtr->DataSize,\r
-                     Data, \r
-                     DataSize, \r
-                     &BufSize\r
+                     Data,\r
+                     DataSize,\r
+                     &MergedBufSize\r
                      );\r
           if (Status == EFI_BUFFER_TOO_SMALL) {\r
             //\r
-            // Signture List is too long, Failed to Append\r
+            // Signature List is too long, Failed to Append.\r
             //\r
             Status = EFI_INVALID_PARAMETER;\r
             goto Done;\r
           }\r
 \r
-          if (BufSize == Variable->CurrPtr->DataSize) {\r
+          if (MergedBufSize == Variable->CurrPtr->DataSize) {\r
             if ((TimeStamp == NULL) || CompareTimeStamp (TimeStamp, &Variable->CurrPtr->TimeStamp)) {\r
               //\r
               // New EFI_SIGNATURE_DATA is not found and timestamp is not later\r
@@ -1959,29 +1962,30 @@ UpdateVariable (
           }\r
         } else {\r
           //\r
-          // For other Variables, append the new data to the end of previous data.\r
+          // For other Variables, append the new data to the end of existing data.\r
           // Max Harware error record variable data size is different from common variable\r
           //\r
           if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) {\r
-            MaxDataSize = PcdGet32 (PcdMaxHardwareErrorVariableSize) - sizeof (VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName));\r
+            MaxDataSize = PcdGet32 (PcdMaxHardwareErrorVariableSize) - DataOffset;\r
           }\r
 \r
           if (Variable->CurrPtr->DataSize + DataSize > MaxDataSize) {\r
             //\r
-            // Exsiting data + Appended data exceed maximum variable size limitation \r
+            // Existing data size + new data size exceed maximum variable size limitation.\r
             //\r
             Status = EFI_INVALID_PARAMETER;\r
             goto Done;\r
           }\r
-          CopyMem ((UINT8*)((UINTN) mStorageArea + Variable->CurrPtr->DataSize), Data, DataSize);\r
-          BufSize = Variable->CurrPtr->DataSize + DataSize;\r
+          CopyMem ((UINT8*) ((UINTN) BufferForMerge + Variable->CurrPtr->DataSize), Data, DataSize);\r
+          MergedBufSize = Variable->CurrPtr->DataSize + DataSize;\r
         }\r
 \r
         //\r
-        // Override Data and DataSize which are used for combined data area including previous and new data.\r
+        // BufferForMerge(from DataOffset of NextVariable) has included the merged existing and new data.\r
         //\r
-        Data     = mStorageArea;\r
-        DataSize = BufSize;\r
+        Data      = BufferForMerge;\r
+        DataSize  = MergedBufSize;\r
+        DataReady = TRUE;\r
       }\r
 \r
       //\r
@@ -2038,9 +2042,7 @@ UpdateVariable (
   //\r
   // Function part - create a new variable and copy the data.\r
   // Both update a variable and create a variable will come here.\r
-\r
-  SetMem (NextVariable, ScratchSize, 0xff);\r
-\r
+  //\r
   NextVariable->StartId     = VARIABLE_DATA;\r
   //\r
   // NextVariable->State = VAR_ADDED;\r
@@ -2082,11 +2084,19 @@ UpdateVariable (
     VarNameSize\r
     );\r
   VarDataOffset = VarNameOffset + VarNameSize + GET_PAD_SIZE (VarNameSize);\r
-  CopyMem (\r
-    (UINT8 *) ((UINTN) NextVariable + VarDataOffset),\r
-    Data,\r
-    DataSize\r
-    );\r
+\r
+  //\r
+  // If DataReady is TRUE, it means the variable data has been saved into\r
+  // NextVariable during EFI_VARIABLE_APPEND_WRITE operation preparation.\r
+  //\r
+  if (!DataReady) {\r
+    CopyMem (\r
+      (UINT8 *) ((UINTN) NextVariable + VarDataOffset),\r
+      Data,\r
+      DataSize\r
+      );\r
+  }\r
+\r
   CopyMem (&NextVariable->VendorGuid, VendorGuid, sizeof (EFI_GUID));\r
   //\r
   // There will be pad bytes after Data, the NextVariable->NameSize and\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableDxe.c
index f1ba9c18f0d18cdeee6c9bf5f745bec0ef7f1004..1d71e4f800d932b4e8722b9567820056d450468d 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableDxe.c
@@ -241,7 +241,6 @@ VariableClassAddressChangeEvent (
   EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->VariableGlobal.VolatileVariableBase);\r
   EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal);\r
   EfiConvertPointer (0x0, (VOID **) &mHashCtx);\r
-  EfiConvertPointer (0x0, (VOID **) &mStorageArea);\r
   EfiConvertPointer (0x0, (VOID **) &mSerializationRuntimeBuffer);\r
   EfiConvertPointer (0x0, (VOID **) &mNvVariableCache);\r
 \r