From: chenc2 Date: Tue, 17 Oct 2017 08:45:06 +0000 (+0800) Subject: SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer. X-Git-Tag: edk2-stable201903~3209 X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=65c77f02104cf0cf7bd224df3a5fc08795df9aad;hp=0085d619ddfb7ef6b702c721d7eb9d33b0ad45aa 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 Cc: Wu Hao A Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: chenc2 Reviewed-by: Chao Zhang --- diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c index 161512a1f4..acb0dc0558 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,12 @@ DeleteSignatureEx ( VariableData = NULL; NewVariableData = NULL; - VariableName = AllocateZeroPool (100); - if (VariableName == NULL) { - Status = EFI_OUT_OF_RESOURCES; - goto ON_EXIT; - } - 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 +3215,6 @@ DeleteSignatureEx ( } ON_EXIT: - SECUREBOOT_FREE_NON_NULL (VariableName); SECUREBOOT_FREE_NON_NULL (VariableData); SECUREBOOT_FREE_NON_NULL (NewVariableData); @@ -3594,9 +3586,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 +3597,6 @@ LoadSignatureList ( EndGotoHandle = NULL; Index = 0; VariableData = NULL; - VariableName = NULL; - NameBuffer = NULL; - HelpBuffer = NULL; // // Initialize the container for dynamic opcodes. @@ -3675,20 +3664,14 @@ 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; - } - 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 +3705,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 +3726,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 +3750,6 @@ LoadSignatureList ( QuestionIdBase + Index++ ); - ZeroMem (NameBuffer, 100); - ZeroMem (HelpBuffer, 100); - RemainingSize -= ListWalker->SignatureListSize; ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize); } @@ -3805,10 +3776,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 +3925,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 +3969,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 +3979,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 +4020,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 +4042,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 +4082,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 +4127,12 @@ 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; - } - 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 +4156,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 +4172,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 +4208,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 +4220,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 e8ae992dd2..f4935965a9 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