From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=chen.a.chen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 18BD021F7D4F0 for ; Fri, 13 Oct 2017 04:12:18 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Oct 2017 04:15:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,370,1503385200"; d="scan'208";a="162275000" Received: from chenche4.ccr.corp.intel.com ([10.239.158.36]) by fmsmga005.fm.intel.com with ESMTP; 13 Oct 2017 04:15:49 -0700 From: chenc2 To: edk2-devel@lists.01.org Cc: chenc2 , Zhang Chao B , Wu Hao A Date: Fri, 13 Oct 2017 19:15:45 +0800 Message-Id: <20171013111545.10776-1-chen.a.chen@intel.com> X-Mailer: git-send-email 2.13.2.windows.1 Subject: [PATCH] SecurityPkg/SecureBootConfigDxe: Change the declaring of buffer. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Oct 2017 11:12:19 -0000 For known max length buffer, declaring there buffers as arrays instead of allocating from the heap. Cc: Zhang Chao B Cc: Wu Hao A Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: chenc2 --- .../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