SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer.

The change doesn't impact the functionality.
To avoid magic code is helpful for maintaining the codes.
Use stack variable for known max length variable is more
clear and safe than heap buffer.

Cc: Zhang Chao B <chao.b.zhang@intel.com>
Cc: Wu Hao A <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: chenc2 <chen.a.chen@intel.com>
Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 161512a1f47aafaeb31e83b7f49ae7b9c6a1105b..acb0dc055892bb6e3a81d2fc086d30e0a079e014 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -3073,7 +3073,7 @@ DeleteSignatureEx (
   EFI_SIGNATURE_LIST  *ListWalker;\r
   EFI_SIGNATURE_LIST  *NewCertList;\r
   EFI_SIGNATURE_DATA  *DataWalker;\r
-  CHAR16              *VariableName;\r
+  CHAR16              VariableName[BUFFER_MAX_SIZE];\r
   UINT32              VariableAttr;\r
   UINTN               VariableDataSize;\r
   UINTN               RemainingSize;\r
@@ -3084,7 +3084,6 @@ DeleteSignatureEx (
   UINT8               *NewVariableData;\r
 \r
   Status              = EFI_SUCCESS;\r
-  VariableName        = NULL;\r
   VariableAttr        = 0;\r
   VariableDataSize    = 0;\r
   ListIndex           = 0;\r
@@ -3092,18 +3091,12 @@ DeleteSignatureEx (
   VariableData        = NULL;\r
   NewVariableData     = NULL;\r
 \r
-  VariableName = AllocateZeroPool (100);\r
-  if (VariableName == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
   if (PrivateData->VariableName == Variable_DB) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE);\r
   } else if (PrivateData->VariableName == Variable_DBX) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE1);\r
   } else if (PrivateData->VariableName == Variable_DBT) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE2);\r
   } else {\r
     goto ON_EXIT;\r
   }\r
@@ -3222,7 +3215,6 @@ DeleteSignatureEx (
   }\r
 \r
 ON_EXIT:\r
-  SECUREBOOT_FREE_NON_NULL (VariableName);\r
   SECUREBOOT_FREE_NON_NULL (VariableData);\r
   SECUREBOOT_FREE_NON_NULL (NewVariableData);\r
 \r
@@ -3594,9 +3586,9 @@ LoadSignatureList (
   UINTN                 RemainingSize;\r
   UINT16                Index;\r
   UINT8                 *VariableData;\r
-  CHAR16                *VariableName;\r
-  CHAR16                *NameBuffer;\r
-  CHAR16                *HelpBuffer;\r
+  CHAR16                VariableName[BUFFER_MAX_SIZE];\r
+  CHAR16                NameBuffer[BUFFER_MAX_SIZE];\r
+  CHAR16                HelpBuffer[BUFFER_MAX_SIZE];\r
 \r
   Status                = EFI_SUCCESS;\r
   StartOpCodeHandle     = NULL;\r
@@ -3605,9 +3597,6 @@ LoadSignatureList (
   EndGotoHandle         = NULL;\r
   Index                 = 0;\r
   VariableData          = NULL;\r
-  VariableName          = NULL;\r
-  NameBuffer            = NULL;\r
-  HelpBuffer            = NULL;\r
 \r
   //\r
   // Initialize the container for dynamic opcodes.\r
@@ -3675,20 +3664,14 @@ LoadSignatureList (
   EndGoto->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;\r
   EndGoto->Number = LABEL_END;\r
 \r
-  VariableName = AllocateZeroPool (100);\r
-  if (VariableName == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
   if (PrivateData->VariableName == Variable_DB) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE);\r
     DstFormId = FORMID_SECURE_BOOT_DB_OPTION_FORM;\r
   } else if (PrivateData->VariableName == Variable_DBX) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE1);\r
     DstFormId = FORMID_SECURE_BOOT_DBX_OPTION_FORM;\r
   } else if (PrivateData->VariableName == Variable_DBT) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE2);\r
     DstFormId = FORMID_SECURE_BOOT_DBT_OPTION_FORM;\r
   } else {\r
     goto ON_EXIT;\r
@@ -3722,18 +3705,6 @@ LoadSignatureList (
     goto ON_EXIT;\r
   }\r
 \r
-  NameBuffer = AllocateZeroPool (100);\r
-  if (NameBuffer == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
-  HelpBuffer = AllocateZeroPool (100);\r
-  if (HelpBuffer == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
   RemainingSize = DataSize;\r
   ListWalker    = (EFI_SIGNATURE_LIST *)VariableData;\r
   while ((RemainingSize > 0) && (RemainingSize >= ListWalker->SignatureListSize)) {\r
@@ -3755,13 +3726,16 @@ LoadSignatureList (
       ListType = STRING_TOKEN (STR_LIST_TYPE_UNKNOWN);\r
     }\r
 \r
+    ZeroMem (NameBuffer, sizeof (NameBuffer));\r
     UnicodeSPrint (NameBuffer,\r
-      100,\r
+      sizeof (NameBuffer),\r
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL),\r
       Index + 1\r
     );\r
+\r
+    ZeroMem (HelpBuffer, sizeof (HelpBuffer));\r
     UnicodeSPrint (HelpBuffer,\r
-      100,\r
+      sizeof (HelpBuffer),\r
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL),\r
       HiiGetString (PrivateData->HiiHandle, ListType, NULL),\r
       SIGNATURE_DATA_COUNTS (ListWalker)\r
@@ -3776,9 +3750,6 @@ LoadSignatureList (
       QuestionIdBase + Index++\r
     );\r
 \r
-    ZeroMem (NameBuffer, 100);\r
-    ZeroMem (HelpBuffer, 100);\r
-\r
     RemainingSize -= ListWalker->SignatureListSize;\r
     ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);\r
   }\r
@@ -3805,10 +3776,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (StartGotoHandle);\r
   SECUREBOOT_FREE_NON_OPCODE (EndGotoHandle);\r
 \r
-  SECUREBOOT_FREE_NON_NULL (VariableName);\r
   SECUREBOOT_FREE_NON_NULL (VariableData);\r
-  SECUREBOOT_FREE_NON_NULL (NameBuffer);\r
-  SECUREBOOT_FREE_NON_NULL (HelpBuffer);\r
 \r
   PrivateData->ListCount = Index;\r
 \r
@@ -3957,18 +3925,16 @@ FormatHelpInfo (
   UINTN           DataSize;\r
   UINTN           HelpInfoIndex;\r
   UINTN           TotalSize;\r
-  CHAR16          *GuidString;\r
+  CHAR16          GuidString[BUFFER_MAX_SIZE];\r
+  CHAR16          TimeString[BUFFER_MAX_SIZE];\r
   CHAR16          *DataString;\r
-  CHAR16          *TimeString;\r
   CHAR16          *HelpInfoString;\r
   BOOLEAN         IsCert;\r
 \r
   Status          = EFI_SUCCESS;\r
   Time            = NULL;\r
   HelpInfoIndex   = 0;\r
-  GuidString      = NULL;\r
   DataString      = NULL;\r
-  TimeString      = NULL;\r
   HelpInfoString  = NULL;\r
   IsCert          = FALSE;\r
 \r
@@ -4003,12 +3969,6 @@ FormatHelpInfo (
     goto ON_EXIT;\r
   }\r
 \r
-  GuidString = AllocateZeroPool (100);\r
-  if (GuidString == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
   TotalSize = 1024;\r
   HelpInfoString = AllocateZeroPool (TotalSize);\r
   if (HelpInfoString == NULL) {\r
@@ -4019,7 +3979,8 @@ FormatHelpInfo (
   //\r
   // Format GUID part.\r
   //\r
-  GuidToString(&DataEntry->SignatureOwner, GuidString, 100);\r
+  ZeroMem (GuidString, sizeof (GuidString));\r
+  GuidToString(&DataEntry->SignatureOwner, GuidString, BUFFER_MAX_SIZE);\r
   HelpInfoIndex += UnicodeSPrint (\r
                      &HelpInfoString[HelpInfoIndex],\r
                      TotalSize - sizeof(CHAR16) * HelpInfoIndex,\r
@@ -4059,15 +4020,10 @@ FormatHelpInfo (
   // Format revocation time part.\r
   //\r
   if (Time != NULL) {\r
-    TimeString = AllocateZeroPool(100);\r
-    if (TimeString == NULL) {\r
-      Status = EFI_OUT_OF_RESOURCES;\r
-      goto ON_EXIT;\r
-    }\r
-\r
+    ZeroMem (TimeString, sizeof (TimeString));\r
     UnicodeSPrint (\r
       TimeString,\r
-      100,\r
+      sizeof (TimeString),\r
       L"%d-%d-%d %d:%d:%d",\r
       Time->Year,\r
       Time->Month,\r
@@ -4086,11 +4042,8 @@ FormatHelpInfo (
   }\r
 \r
   *StringId = HiiSetString (PrivateData->HiiHandle, 0, HelpInfoString, NULL);\r
-\r
 ON_EXIT:\r
-  SECUREBOOT_FREE_NON_NULL (GuidString);\r
   SECUREBOOT_FREE_NON_NULL (DataString);\r
-  SECUREBOOT_FREE_NON_NULL (TimeString);\r
   SECUREBOOT_FREE_NON_NULL (HelpInfoString);\r
 \r
   return Status;\r
@@ -4129,16 +4082,14 @@ LoadSignatureData (
   UINTN                 RemainingSize;\r
   UINT16                Index;\r
   UINT8                 *VariableData;\r
-  CHAR16                *VariableName;\r
-  CHAR16                *NameBuffer;\r
+  CHAR16                VariableName[BUFFER_MAX_SIZE];\r
+  CHAR16                NameBuffer[BUFFER_MAX_SIZE];\r
 \r
   Status              = EFI_SUCCESS;\r
   StartOpCodeHandle   = NULL;\r
   EndOpCodeHandle     = NULL;\r
   Index               = 0;\r
   VariableData        = NULL;\r
-  VariableName        = NULL;\r
-  NameBuffer          = NULL;\r
 \r
   //\r
   // Initialize the container for dynamic opcodes.\r
@@ -4176,18 +4127,12 @@ LoadSignatureData (
   EndLabel->ExtendOpCode  = EFI_IFR_EXTEND_OP_LABEL;\r
   EndLabel->Number        = LABEL_END;\r
 \r
-  VariableName = AllocateZeroPool (100);\r
-  if (VariableName == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
   if (PrivateData->VariableName == Variable_DB) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE);\r
   } else if (PrivateData->VariableName == Variable_DBX) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE1);\r
   } else if (PrivateData->VariableName == Variable_DBT) {\r
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);\r
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE2);\r
   } else {\r
     goto ON_EXIT;\r
   }\r
@@ -4211,12 +4156,6 @@ LoadSignatureData (
     goto ON_EXIT;\r
   }\r
 \r
-  NameBuffer = AllocateZeroPool (100);\r
-  if (NameBuffer == NULL) {\r
-    Status = EFI_OUT_OF_RESOURCES;\r
-    goto ON_EXIT;\r
-  }\r
-\r
   RemainingSize = DataSize;\r
   ListWalker = (EFI_SIGNATURE_LIST *)VariableData;\r
 \r
@@ -4233,8 +4172,9 @@ LoadSignatureData (
     //\r
     // Format name buffer.\r
     //\r
+    ZeroMem (NameBuffer, sizeof (NameBuffer));\r
     UnicodeSPrint (NameBuffer,\r
-      100,\r
+      sizeof (NameBuffer),\r
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL),\r
       Index + 1\r
     );\r
@@ -4268,7 +4208,6 @@ LoadSignatureData (
   // This memory buffer will be freed when exit from the SECUREBOOT_DELETE_SIGNATURE_DATA_FORM form.\r
   //\r
   PrivateData->CheckArray = AllocateZeroPool (SIGNATURE_DATA_COUNTS (ListWalker) * sizeof (BOOLEAN));\r
-\r
 ON_EXIT:\r
   HiiUpdateForm (\r
     PrivateData->HiiHandle,\r
@@ -4281,9 +4220,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (StartOpCodeHandle);\r
   SECUREBOOT_FREE_NON_OPCODE (EndOpCodeHandle);\r
 \r
-  SECUREBOOT_FREE_NON_NULL (VariableName);\r
   SECUREBOOT_FREE_NON_NULL (VariableData);\r
-  SECUREBOOT_FREE_NON_NULL (NameBuffer);\r
 \r
   return Status;\r
 }\r

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index e8ae992dd21762df6454a7bd90002241ac8c2e63..f4935965a9d1f16cb28368b27c8454e4485aa8a1 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
@@ -67,7 +67,7 @@ extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 \r
 #define MAX_CHAR              480\r
 #define TWO_BYTE_ENCODE       0x82\r
-\r
+#define BUFFER_MAX_SIZE       100\r
 \r
 //\r
 // SHA-256 digest size in bytes\r