From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=dandan.bi@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 0F5B3202E53EF for ; Tue, 26 Jun 2018 23:18:29 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 23:18:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,278,1526367600"; d="scan'208";a="235935507" Received: from shwdeopenpsi114.ccr.corp.intel.com ([10.239.157.135]) by orsmga005.jf.intel.com with ESMTP; 26 Jun 2018 23:18:27 -0700 From: Dandan Bi To: edk2-devel@lists.01.org Cc: Liming Gao , Hao Wu Date: Wed, 27 Jun 2018 14:16:23 +0800 Message-Id: <20180627061623.169516-2-dandan.bi@intel.com> X-Mailer: git-send-email 2.14.3.windows.1 In-Reply-To: <20180627061623.169516-1-dandan.bi@intel.com> References: <20180627061623.169516-1-dandan.bi@intel.com> Subject: [patch] MdeModulePkg/PerformanceLib: Add NULL pointer check X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jun 2018 06:18:29 -0000 1. Add NULL pointer check for the "Guid" parameter when handle FPDT_DUAL_GUID_STRING_EVENT_TYPE. 2. Make the code logic in DxeCore/SmmCore/PeiPerformanceLib aligned when handle FPDT_DUAL_GUID_STRING_EVENT_TYPE. Cc: Liming Gao Cc: Hao Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi --- .../DxeCorePerformanceLib/DxeCorePerformanceLib.c | 18 ++++++++++++------ .../Library/PeiPerformanceLib/PeiPerformanceLib.c | 13 ++++++++----- .../SmmCorePerformanceLib/SmmCorePerformanceLib.c | 18 ++++++++++++------ 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c index efff5134c7b..6e0c328c635 100644 --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c @@ -1109,18 +1109,17 @@ InsertFpdtRecord ( case PERF_EVENTSIGNAL_START_ID: case PERF_EVENTSIGNAL_END_ID: case PERF_CALLBACK_START_ID: case PERF_CALLBACK_END_ID: - if (String == NULL) { + if (String == NULL || Guid == NULL) { return EFI_INVALID_PARAMETER; } - // - // Cache the event guid in string event record when PcdEdkiiFpdtStringRecordEnableOnly == TRUE - // - CopyGuid (&ModuleGuid, Guid); StringPtr = String; + if (AsciiStrLen (String) == 0) { + StringPtr = "unknown name"; + } if (!PcdGetBool (PcdEdkiiFpdtStringRecordEnableOnly)) { FpdtRecordPtr.DualGuidStringEvent->Header.Type = FPDT_DUAL_GUID_STRING_EVENT_TYPE; FpdtRecordPtr.DualGuidStringEvent->Header.Length = sizeof (FPDT_DUAL_GUID_STRING_EVENT_RECORD); FpdtRecordPtr.DualGuidStringEvent->Header.Revision = FPDT_RECORD_REVISION_1; FpdtRecordPtr.DualGuidStringEvent->ProgressID = PerfId; @@ -1194,11 +1193,18 @@ InsertFpdtRecord ( FpdtRecordPtr.DynamicStringEvent->Header.Type = FPDT_DYNAMIC_STRING_EVENT_TYPE; FpdtRecordPtr.DynamicStringEvent->Header.Length = sizeof (FPDT_DYNAMIC_STRING_EVENT_RECORD); FpdtRecordPtr.DynamicStringEvent->Header.Revision = FPDT_RECORD_REVISION_1; FpdtRecordPtr.DynamicStringEvent->ProgressID = PerfId; FpdtRecordPtr.DynamicStringEvent->Timestamp = TimeStamp; - CopyMem (&FpdtRecordPtr.DynamicStringEvent->Guid, &ModuleGuid, sizeof (FpdtRecordPtr.DynamicStringEvent->Guid)); + if (Guid != NULL) { + // + // Cache the event guid in string event record. + // + CopyMem (&FpdtRecordPtr.DynamicStringEvent->Guid, Guid, sizeof (FpdtRecordPtr.DynamicStringEvent->Guid)); + } else { + CopyMem (&FpdtRecordPtr.DynamicStringEvent->Guid, &ModuleGuid, sizeof (FpdtRecordPtr.DynamicStringEvent->Guid)); + } if (AsciiStrLen (StringPtr) == 0) { StringPtr = "unknown name"; } CopyStringIntoPerfRecordAndUpdateLength (FpdtRecordPtr.DynamicStringEvent->String, StringPtr, &FpdtRecordPtr.DynamicStringEvent->Header.Length); diff --git a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c index cd1b0e34ef7..3a44a0a438e 100644 --- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c +++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c @@ -75,14 +75,14 @@ GetFpdtRecordPtr ( // // PEI Performance HOB was found, then return the existing one. // PeiFirmwarePerformance = (UINT8*)GET_GUID_HOB_DATA (GuidHob); *PeiPerformanceLogHeader = (FPDT_PEI_EXT_PERF_HEADER *)PeiFirmwarePerformance; - if (!(*PeiPerformanceLogHeader)->HobIsFull && (*PeiPerformanceLogHeader)->SizeOfAllEntries + RecordSize > (UINTN)(PeiPerformanceLogEntries * PEI_MAX_RECORD_SIZE)) { + if (!(*PeiPerformanceLogHeader)->HobIsFull && (*PeiPerformanceLogHeader)->SizeOfAllEntries + RecordSize > (PeiPerformanceLogEntries * PEI_MAX_RECORD_SIZE)) { (*PeiPerformanceLogHeader)->HobIsFull = TRUE; } - if (!(*PeiPerformanceLogHeader)->HobIsFull && (*PeiPerformanceLogHeader)->SizeOfAllEntries + RecordSize <= (UINTN)(PeiPerformanceLogEntries * PEI_MAX_RECORD_SIZE)) { + if (!(*PeiPerformanceLogHeader)->HobIsFull && (*PeiPerformanceLogHeader)->SizeOfAllEntries + RecordSize <= (PeiPerformanceLogEntries * PEI_MAX_RECORD_SIZE)) { FpdtRecordPtr->RecordHeader = (EFI_ACPI_5_0_FPDT_PERFORMANCE_RECORD_HEADER *)(PeiFirmwarePerformance + sizeof (FPDT_PEI_EXT_PERF_HEADER) + (*PeiPerformanceLogHeader)->SizeOfAllEntries); break; } // // Previous HOB is used, then find next one. @@ -421,23 +421,26 @@ InsertFpdtRecord ( case PERF_EVENTSIGNAL_START_ID: case PERF_EVENTSIGNAL_END_ID: case PERF_CALLBACK_START_ID: case PERF_CALLBACK_END_ID: - if (String != NULL && AsciiStrLen (String) != 0) { - StringPtr = String; - } else { + if (String == NULL || Guid == NULL) { + return EFI_INVALID_PARAMETER; + } + StringPtr = String; + if (AsciiStrLen (String) == 0) { StringPtr = "unknown name"; } if (!PcdGetBool (PcdEdkiiFpdtStringRecordEnableOnly)) { FpdtRecordPtr.DualGuidStringEvent->Header.Type = FPDT_DUAL_GUID_STRING_EVENT_TYPE; FpdtRecordPtr.DualGuidStringEvent->Header.Length = sizeof (FPDT_DUAL_GUID_STRING_EVENT_RECORD); FpdtRecordPtr.DualGuidStringEvent->Header.Revision = FPDT_RECORD_REVISION_1; FpdtRecordPtr.DualGuidStringEvent->ProgressID = PerfId; FpdtRecordPtr.DualGuidStringEvent->Timestamp = TimeStamp; CopyMem (&FpdtRecordPtr.DualGuidStringEvent->Guid1, ModuleGuid, sizeof (FpdtRecordPtr.DualGuidStringEvent->Guid1)); CopyMem (&FpdtRecordPtr.DualGuidStringEvent->Guid2, Guid, sizeof (FpdtRecordPtr.DualGuidStringEvent->Guid2)); + CopyStringIntoPerfRecordAndUpdateLength (FpdtRecordPtr.DualGuidStringEvent->String, StringPtr, &FpdtRecordPtr.DualGuidStringEvent->Header.Length); } break; case PERF_EVENT_ID: case PERF_FUNCTION_START_ID: diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c index 0c00fb51e82..f18c3fb60df 100644 --- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c +++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c @@ -647,18 +647,17 @@ InsertFpdtRecord ( case PERF_EVENTSIGNAL_START_ID: case PERF_EVENTSIGNAL_END_ID: case PERF_CALLBACK_START_ID: case PERF_CALLBACK_END_ID: - if (String == NULL) { + if (String == NULL || Guid == NULL) { return EFI_INVALID_PARAMETER; } - // - // Cache the event guid in string event record when PcdEdkiiFpdtStringRecordEnableOnly == TRUE - // - CopyGuid (&ModuleGuid, Guid); StringPtr = String; + if (AsciiStrLen (String) == 0) { + StringPtr = "unknown name"; + } if (!PcdGetBool (PcdEdkiiFpdtStringRecordEnableOnly)) { FpdtRecordPtr.DualGuidStringEvent->Header.Type = FPDT_DUAL_GUID_STRING_EVENT_TYPE; FpdtRecordPtr.DualGuidStringEvent->Header.Length = sizeof (FPDT_DUAL_GUID_STRING_EVENT_RECORD); FpdtRecordPtr.DualGuidStringEvent->Header.Revision = FPDT_RECORD_REVISION_1; FpdtRecordPtr.DualGuidStringEvent->ProgressID = PerfId; @@ -732,11 +731,18 @@ InsertFpdtRecord ( FpdtRecordPtr.DynamicStringEvent->Header.Type = FPDT_DYNAMIC_STRING_EVENT_TYPE; FpdtRecordPtr.DynamicStringEvent->Header.Length = sizeof (FPDT_DYNAMIC_STRING_EVENT_RECORD); FpdtRecordPtr.DynamicStringEvent->Header.Revision = FPDT_RECORD_REVISION_1; FpdtRecordPtr.DynamicStringEvent->ProgressID = PerfId; FpdtRecordPtr.DynamicStringEvent->Timestamp = TimeStamp; - CopyMem (&FpdtRecordPtr.DynamicStringEvent->Guid, &ModuleGuid, sizeof (FpdtRecordPtr.DynamicStringEvent->Guid)); + if (Guid != NULL) { + // + // Cache the event guid in string event record. + // + CopyMem (&FpdtRecordPtr.DynamicStringEvent->Guid, Guid, sizeof (FpdtRecordPtr.DynamicStringEvent->Guid)); + } else { + CopyMem (&FpdtRecordPtr.DynamicStringEvent->Guid, &ModuleGuid, sizeof (FpdtRecordPtr.DynamicStringEvent->Guid)); + } if (AsciiStrLen (StringPtr) == 0) { StringPtr = "unknown name"; } CopyStringIntoPerfRecordAndUpdateLength (FpdtRecordPtr.DynamicStringEvent->String, StringPtr, &FpdtRecordPtr.DynamicStringEvent->Header.Length); -- 2.14.3.windows.1