From: chenc2 <chen.a.chen@intel.com>
To: edk2-devel@lists.01.org
Cc: chenc2 <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.
Date: Fri, 13 Oct 2017 19:15:45 +0800 [thread overview]
Message-ID: <20171013111545.10776-1-chen.a.chen@intel.com> (raw)
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
next reply other threads:[~2017-10-13 11:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-13 11:15 chenc2 [this message]
2017-10-13 15:53 ` [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer Zhang, Chao B
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171013111545.10776-1-chen.a.chen@intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox