public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer.
@ 2017-10-13 11:15 chenc2
  2017-10-13 15:53 ` Zhang, Chao B
  0 siblings, 1 reply; 2+ messages in thread
From: chenc2 @ 2017-10-13 11:15 UTC (permalink / raw)
  To: edk2-devel; +Cc: chenc2, Zhang Chao B, Wu Hao A

For known max length buffer, declaring there
buffers as arrays instead of allocating from the heap.

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>
---
 .../SecureBootConfigDxe/SecureBootConfigImpl.c     | 122 ++++++---------------
 .../SecureBootConfigDxe/SecureBootConfigImpl.h     |   2 +-
 2 files changed, 32 insertions(+), 92 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 3e03be9738..3ff90b5c8b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -3073,7 +3073,7 @@ DeleteSignatureEx (
   EFI_SIGNATURE_LIST  *ListWalker;
   EFI_SIGNATURE_LIST  *NewCertList;
   EFI_SIGNATURE_DATA  *DataWalker;
-  CHAR16              *VariableName;
+  CHAR16              VariableName[BUFFER_MAX_SIZE];
   UINT32              VariableAttr;
   UINTN               VariableDataSize;
   UINTN               RemainingSize;
@@ -3084,7 +3084,6 @@ DeleteSignatureEx (
   UINT8               *NewVariableData;
 
   Status              = EFI_SUCCESS;
-  VariableName        = NULL;
   VariableAttr        = 0;
   VariableDataSize    = 0;
   ListIndex           = 0;
@@ -3092,18 +3091,13 @@ DeleteSignatureEx (
   VariableData        = NULL;
   NewVariableData     = NULL;
 
-  VariableName = AllocateZeroPool (100);
-  if (VariableName == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
+  ZeroMem (VariableName, sizeof (VariableName));
   if (PrivateData->VariableName == VARIABLE_DB) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE);
   } else if (PrivateData->VariableName == VARIABLE_DBX) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE1);
   } else if (PrivateData->VariableName == VARIABLE_DBT) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE2);
   } else {
     goto ON_EXIT;
   }
@@ -3222,7 +3216,6 @@ DeleteSignatureEx (
   }
 
 ON_EXIT:
-  SECUREBOOT_FREE_NON_NULL (VariableName);
   SECUREBOOT_FREE_NON_NULL (VariableData);
   SECUREBOOT_FREE_NON_NULL (NewVariableData);
 
@@ -3594,9 +3587,9 @@ LoadSignatureList (
   UINTN                 RemainingSize;
   UINT16                Index;
   UINT8                 *VariableData;
-  CHAR16                *VariableName;
-  CHAR16                *NameBuffer;
-  CHAR16                *HelpBuffer;
+  CHAR16                VariableName[BUFFER_MAX_SIZE];
+  CHAR16                NameBuffer[BUFFER_MAX_SIZE];
+  CHAR16                HelpBuffer[BUFFER_MAX_SIZE];
 
   Status                = EFI_SUCCESS;
   StartOpCodeHandle     = NULL;
@@ -3605,9 +3598,6 @@ LoadSignatureList (
   EndGotoHandle         = NULL;
   Index                 = 0;
   VariableData          = NULL;
-  VariableName          = NULL;
-  NameBuffer            = NULL;
-  HelpBuffer            = NULL;
 
   //
   // Initialize the container for dynamic opcodes.
@@ -3675,20 +3665,15 @@ LoadSignatureList (
   EndGoto->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
   EndGoto->Number = LABEL_END;
 
-  VariableName = AllocateZeroPool (100);
-  if (VariableName == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
+  ZeroMem (VariableName, sizeof (VariableName));
   if (PrivateData->VariableName == VARIABLE_DB) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE);
     DstFormId = FORMID_SECURE_BOOT_DB_OPTION_FORM;
   } else if (PrivateData->VariableName == VARIABLE_DBX) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE1);
     DstFormId = FORMID_SECURE_BOOT_DBX_OPTION_FORM;
   } else if (PrivateData->VariableName == VARIABLE_DBT) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE2);
     DstFormId = FORMID_SECURE_BOOT_DBT_OPTION_FORM;
   } else {
     goto ON_EXIT;
@@ -3722,18 +3707,6 @@ LoadSignatureList (
     goto ON_EXIT;
   }
 
-  NameBuffer = AllocateZeroPool (100);
-  if (NameBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
-  HelpBuffer = AllocateZeroPool (100);
-  if (HelpBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
   RemainingSize = DataSize;
   ListWalker    = (EFI_SIGNATURE_LIST *)VariableData;
   while ((RemainingSize > 0) && (RemainingSize >= ListWalker->SignatureListSize)) {
@@ -3755,13 +3728,16 @@ LoadSignatureList (
       ListType = STRING_TOKEN (STR_LIST_TYPE_UNKNOWN);
     }
 
+    ZeroMem (NameBuffer, sizeof (NameBuffer));
     UnicodeSPrint (NameBuffer,
-      100,
+      sizeof (NameBuffer),
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL),
       Index + 1
     );
+
+    ZeroMem (HelpBuffer, sizeof (HelpBuffer));
     UnicodeSPrint (HelpBuffer,
-      100,
+      sizeof (HelpBuffer),
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL),
       HiiGetString (PrivateData->HiiHandle, ListType, NULL),
       SIGNATURE_DATA_COUNTS (ListWalker)
@@ -3776,9 +3752,6 @@ LoadSignatureList (
       QuestionIdBase + Index++
     );
 
-    ZeroMem (NameBuffer, 100);
-    ZeroMem (HelpBuffer, 100);
-
     RemainingSize -= ListWalker->SignatureListSize;
     ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);
   }
@@ -3805,10 +3778,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (StartGotoHandle);
   SECUREBOOT_FREE_NON_OPCODE (EndGotoHandle);
 
-  SECUREBOOT_FREE_NON_NULL (VariableName);
   SECUREBOOT_FREE_NON_NULL (VariableData);
-  SECUREBOOT_FREE_NON_NULL (NameBuffer);
-  SECUREBOOT_FREE_NON_NULL (HelpBuffer);
 
   PrivateData->ListCount = Index;
 
@@ -3957,18 +3927,16 @@ FormatHelpInfo (
   UINTN           DataSize;
   UINTN           HelpInfoIndex;
   UINTN           TotalSize;
-  CHAR16          *GuidString;
+  CHAR16          GuidString[BUFFER_MAX_SIZE];
+  CHAR16          TimeString[BUFFER_MAX_SIZE];
   CHAR16          *DataString;
-  CHAR16          *TimeString;
   CHAR16          *HelpInfoString;
   BOOLEAN         IsCert;
 
   Status          = EFI_SUCCESS;
   Time            = NULL;
   HelpInfoIndex   = 0;
-  GuidString      = NULL;
   DataString      = NULL;
-  TimeString      = NULL;
   HelpInfoString  = NULL;
   IsCert          = FALSE;
 
@@ -4003,12 +3971,6 @@ FormatHelpInfo (
     goto ON_EXIT;
   }
 
-  GuidString = AllocateZeroPool (100);
-  if (GuidString == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
   TotalSize = 1024;
   HelpInfoString = AllocateZeroPool (TotalSize);
   if (HelpInfoString == NULL) {
@@ -4019,7 +3981,8 @@ FormatHelpInfo (
   //
   // Format GUID part.
   //
-  GuidToString(&DataEntry->SignatureOwner, GuidString, 100);
+  ZeroMem (GuidString, sizeof (GuidString));
+  GuidToString(&DataEntry->SignatureOwner, GuidString, BUFFER_MAX_SIZE);
   HelpInfoIndex += UnicodeSPrint (
                      &HelpInfoString[HelpInfoIndex],
                      TotalSize - sizeof(CHAR16) * HelpInfoIndex,
@@ -4059,15 +4022,10 @@ FormatHelpInfo (
   // Format revocation time part.
   //
   if (Time != NULL) {
-    TimeString = AllocateZeroPool(100);
-    if (TimeString == NULL) {
-      Status = EFI_OUT_OF_RESOURCES;
-      goto ON_EXIT;
-    }
-
+    ZeroMem (TimeString, sizeof (TimeString));
     UnicodeSPrint (
       TimeString,
-      100,
+      sizeof (TimeString),
       L"%d-%d-%d %d:%d:%d",
       Time->Year,
       Time->Month,
@@ -4086,11 +4044,8 @@ FormatHelpInfo (
   }
 
   *StringId = HiiSetString (PrivateData->HiiHandle, 0, HelpInfoString, NULL);
-
 ON_EXIT:
-  SECUREBOOT_FREE_NON_NULL (GuidString);
   SECUREBOOT_FREE_NON_NULL (DataString);
-  SECUREBOOT_FREE_NON_NULL (TimeString);
   SECUREBOOT_FREE_NON_NULL (HelpInfoString);
 
   return Status;
@@ -4129,16 +4084,14 @@ LoadSignatureData (
   UINTN                 RemainingSize;
   UINT16                Index;
   UINT8                 *VariableData;
-  CHAR16                *VariableName;
-  CHAR16                *NameBuffer;
+  CHAR16                VariableName[BUFFER_MAX_SIZE];
+  CHAR16                NameBuffer[BUFFER_MAX_SIZE];
 
   Status              = EFI_SUCCESS;
   StartOpCodeHandle   = NULL;
   EndOpCodeHandle     = NULL;
   Index               = 0;
   VariableData        = NULL;
-  VariableName        = NULL;
-  NameBuffer          = NULL;
 
   //
   // Initialize the container for dynamic opcodes.
@@ -4176,18 +4129,13 @@ LoadSignatureData (
   EndLabel->ExtendOpCode  = EFI_IFR_EXTEND_OP_LABEL;
   EndLabel->Number        = LABEL_END;
 
-  VariableName = AllocateZeroPool (100);
-  if (VariableName == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
+  ZeroMem (VariableName, sizeof (VariableName));
   if (PrivateData->VariableName == VARIABLE_DB) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE);
   } else if (PrivateData->VariableName == VARIABLE_DBX) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE1);
   } else if (PrivateData->VariableName == VARIABLE_DBT) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);
+    UnicodeSPrint (VariableName, sizeof (VariableName), EFI_IMAGE_SECURITY_DATABASE2);
   } else {
     goto ON_EXIT;
   }
@@ -4211,12 +4159,6 @@ LoadSignatureData (
     goto ON_EXIT;
   }
 
-  NameBuffer = AllocateZeroPool (100);
-  if (NameBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
   RemainingSize = DataSize;
   ListWalker = (EFI_SIGNATURE_LIST *)VariableData;
 
@@ -4233,8 +4175,9 @@ LoadSignatureData (
     //
     // Format name buffer.
     //
+    ZeroMem (NameBuffer, sizeof (NameBuffer));
     UnicodeSPrint (NameBuffer,
-      100,
+      sizeof (NameBuffer),
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL),
       Index + 1
     );
@@ -4268,7 +4211,6 @@ LoadSignatureData (
   // This memory buffer will be freed when exit from the SECUREBOOT_DELETE_SIGNATURE_DATA_FORM form.
   //
   PrivateData->CheckArray = AllocateZeroPool (SIGNATURE_DATA_COUNTS (ListWalker) * sizeof (BOOLEAN));
-
 ON_EXIT:
   HiiUpdateForm (
     PrivateData->HiiHandle,
@@ -4281,9 +4223,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (StartOpCodeHandle);
   SECUREBOOT_FREE_NON_OPCODE (EndOpCodeHandle);
 
-  SECUREBOOT_FREE_NON_NULL (VariableName);
   SECUREBOOT_FREE_NON_NULL (VariableData);
-  SECUREBOOT_FREE_NON_NULL (NameBuffer);
 
   return Status;
 }
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index 52ad91b002..6082fb7df2 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
@@ -67,7 +67,7 @@ extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 
 #define MAX_CHAR              480
 #define TWO_BYTE_ENCODE       0x82
-
+#define BUFFER_MAX_SIZE       100
 
 //
 // SHA-256 digest size in bytes
-- 
2.13.2.windows.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer.
  2017-10-13 11:15 [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer chenc2
@ 2017-10-13 15:53 ` Zhang, Chao B
  0 siblings, 0 replies; 2+ messages in thread
From: Zhang, Chao B @ 2017-10-13 15:53 UTC (permalink / raw)
  To: Chen, Chen A, edk2-devel@lists.01.org; +Cc: Wu, Hao A

Hi ChenChen:
    It is good to replace magic code with Micro. Can you specify the reason why use Stack instead of Heap?

-----Original Message-----
From: Chen, Chen A 
Sent: Friday, October 13, 2017 7:16 PM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A <chen.a.chen@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer.

For known max length buffer, declaring there buffers as arrays instead of allocating from the heap.

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>
---
 .../SecureBootConfigDxe/SecureBootConfigImpl.c     | 122 ++++++---------------
 .../SecureBootConfigDxe/SecureBootConfigImpl.h     |   2 +-
 2 files changed, 32 insertions(+), 92 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 3e03be9738..3ff90b5c8b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.c
@@ -3073,7 +3073,7 @@ DeleteSignatureEx (
   EFI_SIGNATURE_LIST  *ListWalker;
   EFI_SIGNATURE_LIST  *NewCertList;
   EFI_SIGNATURE_DATA  *DataWalker;
-  CHAR16              *VariableName;
+  CHAR16              VariableName[BUFFER_MAX_SIZE];
   UINT32              VariableAttr;
   UINTN               VariableDataSize;
   UINTN               RemainingSize;
@@ -3084,7 +3084,6 @@ DeleteSignatureEx (
   UINT8               *NewVariableData;
 
   Status              = EFI_SUCCESS;
-  VariableName        = NULL;
   VariableAttr        = 0;
   VariableDataSize    = 0;
   ListIndex           = 0;
@@ -3092,18 +3091,13 @@ DeleteSignatureEx (
   VariableData        = NULL;
   NewVariableData     = NULL;
 
-  VariableName = AllocateZeroPool (100);
-  if (VariableName == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
+  ZeroMem (VariableName, sizeof (VariableName));
   if (PrivateData->VariableName == VARIABLE_DB) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE);
   } else if (PrivateData->VariableName == VARIABLE_DBX) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE1);
   } else if (PrivateData->VariableName == VARIABLE_DBT) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE2);
   } else {
     goto ON_EXIT;
   }
@@ -3222,7 +3216,6 @@ DeleteSignatureEx (
   }
 
 ON_EXIT:
-  SECUREBOOT_FREE_NON_NULL (VariableName);
   SECUREBOOT_FREE_NON_NULL (VariableData);
   SECUREBOOT_FREE_NON_NULL (NewVariableData);
 
@@ -3594,9 +3587,9 @@ LoadSignatureList (
   UINTN                 RemainingSize;
   UINT16                Index;
   UINT8                 *VariableData;
-  CHAR16                *VariableName;
-  CHAR16                *NameBuffer;
-  CHAR16                *HelpBuffer;
+  CHAR16                VariableName[BUFFER_MAX_SIZE];
+  CHAR16                NameBuffer[BUFFER_MAX_SIZE];
+  CHAR16                HelpBuffer[BUFFER_MAX_SIZE];
 
   Status                = EFI_SUCCESS;
   StartOpCodeHandle     = NULL;
@@ -3605,9 +3598,6 @@ LoadSignatureList (
   EndGotoHandle         = NULL;
   Index                 = 0;
   VariableData          = NULL;
-  VariableName          = NULL;
-  NameBuffer            = NULL;
-  HelpBuffer            = NULL;
 
   //
   // Initialize the container for dynamic opcodes.
@@ -3675,20 +3665,15 @@ LoadSignatureList (
   EndGoto->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
   EndGoto->Number = LABEL_END;
 
-  VariableName = AllocateZeroPool (100);
-  if (VariableName == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
+  ZeroMem (VariableName, sizeof (VariableName));
   if (PrivateData->VariableName == VARIABLE_DB) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE);
     DstFormId = FORMID_SECURE_BOOT_DB_OPTION_FORM;
   } else if (PrivateData->VariableName == VARIABLE_DBX) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE1);
     DstFormId = FORMID_SECURE_BOOT_DBX_OPTION_FORM;
   } else if (PrivateData->VariableName == VARIABLE_DBT) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE2);
     DstFormId = FORMID_SECURE_BOOT_DBT_OPTION_FORM;
   } else {
     goto ON_EXIT;
@@ -3722,18 +3707,6 @@ LoadSignatureList (
     goto ON_EXIT;
   }
 
-  NameBuffer = AllocateZeroPool (100);
-  if (NameBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
-  HelpBuffer = AllocateZeroPool (100);
-  if (HelpBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
   RemainingSize = DataSize;
   ListWalker    = (EFI_SIGNATURE_LIST *)VariableData;
   while ((RemainingSize > 0) && (RemainingSize >= ListWalker->SignatureListSize)) { @@ -3755,13 +3728,16 @@ LoadSignatureList (
       ListType = STRING_TOKEN (STR_LIST_TYPE_UNKNOWN);
     }
 
+    ZeroMem (NameBuffer, sizeof (NameBuffer));
     UnicodeSPrint (NameBuffer,
-      100,
+      sizeof (NameBuffer),
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL),
       Index + 1
     );
+
+    ZeroMem (HelpBuffer, sizeof (HelpBuffer));
     UnicodeSPrint (HelpBuffer,
-      100,
+      sizeof (HelpBuffer),
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL),
       HiiGetString (PrivateData->HiiHandle, ListType, NULL),
       SIGNATURE_DATA_COUNTS (ListWalker) @@ -3776,9 +3752,6 @@ LoadSignatureList (
       QuestionIdBase + Index++
     );
 
-    ZeroMem (NameBuffer, 100);
-    ZeroMem (HelpBuffer, 100);
-
     RemainingSize -= ListWalker->SignatureListSize;
     ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);
   }
@@ -3805,10 +3778,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (StartGotoHandle);
   SECUREBOOT_FREE_NON_OPCODE (EndGotoHandle);
 
-  SECUREBOOT_FREE_NON_NULL (VariableName);
   SECUREBOOT_FREE_NON_NULL (VariableData);
-  SECUREBOOT_FREE_NON_NULL (NameBuffer);
-  SECUREBOOT_FREE_NON_NULL (HelpBuffer);
 
   PrivateData->ListCount = Index;
 
@@ -3957,18 +3927,16 @@ FormatHelpInfo (
   UINTN           DataSize;
   UINTN           HelpInfoIndex;
   UINTN           TotalSize;
-  CHAR16          *GuidString;
+  CHAR16          GuidString[BUFFER_MAX_SIZE];
+  CHAR16          TimeString[BUFFER_MAX_SIZE];
   CHAR16          *DataString;
-  CHAR16          *TimeString;
   CHAR16          *HelpInfoString;
   BOOLEAN         IsCert;
 
   Status          = EFI_SUCCESS;
   Time            = NULL;
   HelpInfoIndex   = 0;
-  GuidString      = NULL;
   DataString      = NULL;
-  TimeString      = NULL;
   HelpInfoString  = NULL;
   IsCert          = FALSE;
 
@@ -4003,12 +3971,6 @@ FormatHelpInfo (
     goto ON_EXIT;
   }
 
-  GuidString = AllocateZeroPool (100);
-  if (GuidString == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
   TotalSize = 1024;
   HelpInfoString = AllocateZeroPool (TotalSize);
   if (HelpInfoString == NULL) {
@@ -4019,7 +3981,8 @@ FormatHelpInfo (
   //
   // Format GUID part.
   //
-  GuidToString(&DataEntry->SignatureOwner, GuidString, 100);
+  ZeroMem (GuidString, sizeof (GuidString));  
+ GuidToString(&DataEntry->SignatureOwner, GuidString, BUFFER_MAX_SIZE);
   HelpInfoIndex += UnicodeSPrint (
                      &HelpInfoString[HelpInfoIndex],
                      TotalSize - sizeof(CHAR16) * HelpInfoIndex, @@ -4059,15 +4022,10 @@ FormatHelpInfo (
   // Format revocation time part.
   //
   if (Time != NULL) {
-    TimeString = AllocateZeroPool(100);
-    if (TimeString == NULL) {
-      Status = EFI_OUT_OF_RESOURCES;
-      goto ON_EXIT;
-    }
-
+    ZeroMem (TimeString, sizeof (TimeString));
     UnicodeSPrint (
       TimeString,
-      100,
+      sizeof (TimeString),
       L"%d-%d-%d %d:%d:%d",
       Time->Year,
       Time->Month,
@@ -4086,11 +4044,8 @@ FormatHelpInfo (
   }
 
   *StringId = HiiSetString (PrivateData->HiiHandle, 0, HelpInfoString, NULL);
-
 ON_EXIT:
-  SECUREBOOT_FREE_NON_NULL (GuidString);
   SECUREBOOT_FREE_NON_NULL (DataString);
-  SECUREBOOT_FREE_NON_NULL (TimeString);
   SECUREBOOT_FREE_NON_NULL (HelpInfoString);
 
   return Status;
@@ -4129,16 +4084,14 @@ LoadSignatureData (
   UINTN                 RemainingSize;
   UINT16                Index;
   UINT8                 *VariableData;
-  CHAR16                *VariableName;
-  CHAR16                *NameBuffer;
+  CHAR16                VariableName[BUFFER_MAX_SIZE];
+  CHAR16                NameBuffer[BUFFER_MAX_SIZE];
 
   Status              = EFI_SUCCESS;
   StartOpCodeHandle   = NULL;
   EndOpCodeHandle     = NULL;
   Index               = 0;
   VariableData        = NULL;
-  VariableName        = NULL;
-  NameBuffer          = NULL;
 
   //
   // Initialize the container for dynamic opcodes.
@@ -4176,18 +4129,13 @@ LoadSignatureData (
   EndLabel->ExtendOpCode  = EFI_IFR_EXTEND_OP_LABEL;
   EndLabel->Number        = LABEL_END;
 
-  VariableName = AllocateZeroPool (100);
-  if (VariableName == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
+  ZeroMem (VariableName, sizeof (VariableName));
   if (PrivateData->VariableName == VARIABLE_DB) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE);
   } else if (PrivateData->VariableName == VARIABLE_DBX) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE1);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE1);
   } else if (PrivateData->VariableName == VARIABLE_DBT) {
-    UnicodeSPrint (VariableName, 100, EFI_IMAGE_SECURITY_DATABASE2);
+    UnicodeSPrint (VariableName, sizeof (VariableName), 
+ EFI_IMAGE_SECURITY_DATABASE2);
   } else {
     goto ON_EXIT;
   }
@@ -4211,12 +4159,6 @@ LoadSignatureData (
     goto ON_EXIT;
   }
 
-  NameBuffer = AllocateZeroPool (100);
-  if (NameBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
   RemainingSize = DataSize;
   ListWalker = (EFI_SIGNATURE_LIST *)VariableData;
 
@@ -4233,8 +4175,9 @@ LoadSignatureData (
     //
     // Format name buffer.
     //
+    ZeroMem (NameBuffer, sizeof (NameBuffer));
     UnicodeSPrint (NameBuffer,
-      100,
+      sizeof (NameBuffer),
       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL),
       Index + 1
     );
@@ -4268,7 +4211,6 @@ LoadSignatureData (
   // This memory buffer will be freed when exit from the SECUREBOOT_DELETE_SIGNATURE_DATA_FORM form.
   //
   PrivateData->CheckArray = AllocateZeroPool (SIGNATURE_DATA_COUNTS (ListWalker) * sizeof (BOOLEAN));
-
 ON_EXIT:
   HiiUpdateForm (
     PrivateData->HiiHandle,
@@ -4281,9 +4223,7 @@ ON_EXIT:
   SECUREBOOT_FREE_NON_OPCODE (StartOpCodeHandle);
   SECUREBOOT_FREE_NON_OPCODE (EndOpCodeHandle);
 
-  SECUREBOOT_FREE_NON_NULL (VariableName);
   SECUREBOOT_FREE_NON_NULL (VariableData);
-  SECUREBOOT_FREE_NON_NULL (NameBuffer);
 
   return Status;
 }
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index 52ad91b002..6082fb7df2 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.h
@@ -67,7 +67,7 @@ extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 
 #define MAX_CHAR              480
 #define TWO_BYTE_ENCODE       0x82
-
+#define BUFFER_MAX_SIZE       100
 
 //
 // SHA-256 digest size in bytes
--
2.13.2.windows.1



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-10-13 15:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-13 11:15 [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer chenc2
2017-10-13 15:53 ` Zhang, Chao B

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox