From 540e935e17e3366a0a7e82e0325e00036e488be8 Mon Sep 17 00:00:00 2001 From: kuqin12 <42554914+kuqin12@users.noreply.github.com> Date: Wed, 28 Aug 2024 13:02:10 -0700 Subject: [PATCH] Fix override from CodeQL and cherry-picks from latest basecore (#331) ## Description This change updated the override validation tags. It also integrates the cherry-pick commit from https://github.com/microsoft/mu_basecore/commit/4d722ac91608d3c4fbe807b418b77f2a6e279117. For details on how to complete to complete these options and their meaning refer to [CONTRIBUTING.md](https://github.com/microsoft/mu/blob/HEAD/CONTRIBUTING.md). - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested This was tested on pipeline and passed override check. Also booted on Q35 branch. ## Integration Instructions The change needs to pair with basecore commit https://github.com/microsoft/mu_basecore/commit/4d722ac91608d3c4fbe807b418b77f2a6e279117 or later. --- MmSupervisorPkg/Core/MmSupervisorCore.inf | 7 +- .../Core/Services/MpService/MpService.c | 10 +-- .../Core/Services/MpService/MpService.h | 6 +- .../Core/Services/MpService/SyncTimer.c | 19 ++++-- .../Drivers/MmPeiLaunchers/MmDxeSupport.inf | 2 +- .../Drivers/MmPeiLaunchers/MmIplPei.inf | 4 +- .../MmSupervisorRing3Broker.inf | 4 +- .../Library/BaseLibSysCall/BaseLib.inf | 2 +- .../Library/BaseLibSysCall/String.c | 64 ++++++++++++++----- .../MmiHandlerProfileInfo.c | 9 +-- .../MmiHandlerProfileInfo.inf | 2 +- 11 files changed, 88 insertions(+), 41 deletions(-) diff --git a/MmSupervisorPkg/Core/MmSupervisorCore.inf b/MmSupervisorPkg/Core/MmSupervisorCore.inf index eceafa5c..ab9eb450 100644 --- a/MmSupervisorPkg/Core/MmSupervisorCore.inf +++ b/MmSupervisorPkg/Core/MmSupervisorCore.inf @@ -10,9 +10,9 @@ # ## -#Override : 00000002 | StandaloneMmPkg/Core/StandaloneMmCore.inf | 0958e6b72fe8eeaf87f2d0dd61ab4a62 | 2024-07-29T17-47-37 | 5bfab09d1f243366d256ed254ded0413d9b1440d -#Override : 00000002 | UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 8f4d9ebac99e18a84fe18f790793183e | 2024-07-26T00-52-03 | 5bfab09d1f243366d256ed254ded0413d9b1440d -#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 065a21955cc57cf1c06dabf3d238f34a | 2024-07-29T17-59-14 | 5bfab09d1f243366d256ed254ded0413d9b1440d +#Override : 00000002 | StandaloneMmPkg/Core/StandaloneMmCore.inf | 22aa8d1b884e477e1e078b485974dc90 | 2024-08-28T16-51-15 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c +#Override : 00000002 | UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 465d5d5aecd11469c7b706462e194f94 | 2024-08-28T16-50-23 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c +#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | b775d90b4549dbd378f23de48a0f3a16 | 2024-08-28T16-42-21 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c [Defines] INF_VERSION = 0x0001001A @@ -183,6 +183,7 @@ gMmSupervisorPkgTokenSpaceGuid.PcdEnableSyscallLogs ## CONSUMES [FixedPcd] + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout2 ## CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOMETIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileSize ## SOMETIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize ## CONSUMES diff --git a/MmSupervisorPkg/Core/Services/MpService/MpService.c b/MmSupervisorPkg/Core/Services/MpService/MpService.c index 69fb0b58..a771b0e0 100644 --- a/MmSupervisorPkg/Core/Services/MpService/MpService.c +++ b/MmSupervisorPkg/Core/Services/MpService/MpService.c @@ -1,7 +1,7 @@ /** @file SMM MP service implementation -Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.
Copyright (c) 2017, AMD Incorporated. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent @@ -318,7 +318,7 @@ SmmWaitForApArrival ( // Sync with APs 1st timeout // for (Timer = StartSyncTimer (); - !IsSyncTimerTimeout (Timer) && !(LmceEn && LmceSignal); + !IsSyncTimerTimeout (Timer, mTimeoutTicker) && !(LmceEn && LmceSignal); ) { mSmmMpSyncData->AllApArrivedWithException = AllCpusInSmmExceptBlockedDisabled (); @@ -359,7 +359,7 @@ SmmWaitForApArrival ( // Sync with APs 2nd timeout. // for (Timer = StartSyncTimer (); - !IsSyncTimerTimeout (Timer); + !IsSyncTimerTimeout (Timer, mTimeoutTicker2); ) { mSmmMpSyncData->AllApArrivedWithException = AllCpusInSmmExceptBlockedDisabled (); @@ -772,7 +772,7 @@ APHandler ( // Timeout BSP // for (Timer = StartSyncTimer (); - !IsSyncTimerTimeout (Timer) && + !IsSyncTimerTimeout (Timer, mTimeoutTicker) && !(*mSmmMpSyncData->InsideSmm); ) { @@ -800,7 +800,7 @@ APHandler ( // Now clock BSP for the 2nd time // for (Timer = StartSyncTimer (); - !IsSyncTimerTimeout (Timer) && + !IsSyncTimerTimeout (Timer, mTimeoutTicker2) && !(*mSmmMpSyncData->InsideSmm); ) { diff --git a/MmSupervisorPkg/Core/Services/MpService/MpService.h b/MmSupervisorPkg/Core/Services/MpService/MpService.h index a229c878..16b7b33d 100644 --- a/MmSupervisorPkg/Core/Services/MpService/MpService.h +++ b/MmSupervisorPkg/Core/Services/MpService/MpService.h @@ -109,6 +109,9 @@ typedef struct { extern SMM_DISPATCHER_MP_SYNC_DATA *mSmmMpSyncData; extern UINT64 gPhyMask; +extern UINT64 mTimeoutTicker; +extern UINT64 mTimeoutTicker2; + /** Schedule a procedure to run on the specified CPU. @@ -208,7 +211,8 @@ StartSyncTimer ( BOOLEAN EFIAPI IsSyncTimerTimeout ( - IN UINT64 Timer + IN UINT64 Timer, + IN UINT64 Timeout ); /** diff --git a/MmSupervisorPkg/Core/Services/MpService/SyncTimer.c b/MmSupervisorPkg/Core/Services/MpService/SyncTimer.c index ef221e18..0cbd352e 100644 --- a/MmSupervisorPkg/Core/Services/MpService/SyncTimer.c +++ b/MmSupervisorPkg/Core/Services/MpService/SyncTimer.c @@ -1,7 +1,7 @@ /** @file SMM Timer feature support -Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -12,6 +12,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include UINT64 mTimeoutTicker = 0; + +UINT64 mTimeoutTicker2 = 0; + // // Number of counts in a roll-over cycle of the performance counter. // @@ -39,6 +42,10 @@ InitializeSmmTimer ( MultU64x64 (TimerFrequency, PcdGet64 (PcdCpuSmmApSyncTimeout)), 1000 * 1000 ); + mTimeoutTicker2 = DivU64x32 ( + MultU64x64 (TimerFrequency, PcdGet64 (PcdCpuSmmApSyncTimeout2)), + 1000 * 1000 + ); if (End < Start) { mCountDown = TRUE; mCycle = Start - End; @@ -62,15 +69,17 @@ StartSyncTimer ( } /** - Check if the SMM AP Sync timer is timeout. + Check if the SMM AP Sync Timer is timeout specified by Timeout. - @param Timer The start timer from the begin. + @param Timer The start timer from the begin. + @param Timeout The timeout ticker to wait. **/ BOOLEAN EFIAPI IsSyncTimerTimeout ( - IN UINT64 Timer + IN UINT64 Timer, + IN UINT64 Timeout ) { UINT64 CurrentTimer; @@ -108,5 +117,5 @@ IsSyncTimerTimeout ( } } - return (BOOLEAN)(Delta >= mTimeoutTicker); + return (BOOLEAN)(Delta >= Timeout); } diff --git a/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmDxeSupport.inf b/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmDxeSupport.inf index 0ee24f87..41be3091 100644 --- a/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmDxeSupport.inf +++ b/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmDxeSupport.inf @@ -17,7 +17,7 @@ PI_SPECIFICATION_VERSION = 0x0001000A ENTRY_POINT = MmDxeSupportEntry -#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | b8cd1e0325ad1ee8724c96b07614b9de | 2024-07-29T18-40-40 | 5bfab09d1f243366d256ed254ded0413d9b1440d +#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 2e109ab26b73f3510a952e0c82b14628 | 2024-08-28T16-44-39 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c # # The following information is for reference only and not required by the build tools. diff --git a/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplPei.inf b/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplPei.inf index 6dddbdba..65d7c294 100644 --- a/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplPei.inf +++ b/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplPei.inf @@ -17,8 +17,8 @@ PI_SPECIFICATION_VERSION = 0x0001000A ENTRY_POINT = MmIplPeiEntry -#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | b8cd1e0325ad1ee8724c96b07614b9de | 2024-07-29T18-40-40 | 5bfab09d1f243366d256ed254ded0413d9b1440d -#Override : 00000002 | MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 054eaebaa51932c1c0d916079f932e4e | 2024-07-29T18-47-52 | 5bfab09d1f243366d256ed254ded0413d9b1440d +#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 2e109ab26b73f3510a952e0c82b14628 | 2024-08-28T16-44-39 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c +#Override : 00000002 | MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 21106e057e22a548508f22875b3c2124 | 2024-08-28T17-14-15 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c # # The following information is for reference only and not required by the build tools. diff --git a/MmSupervisorPkg/Drivers/MmSupervisorRing3Broker/MmSupervisorRing3Broker.inf b/MmSupervisorPkg/Drivers/MmSupervisorRing3Broker/MmSupervisorRing3Broker.inf index c188c422..c63cead3 100644 --- a/MmSupervisorPkg/Drivers/MmSupervisorRing3Broker/MmSupervisorRing3Broker.inf +++ b/MmSupervisorPkg/Drivers/MmSupervisorRing3Broker/MmSupervisorRing3Broker.inf @@ -9,8 +9,8 @@ #**/ # This module contains an instance of protocol/handle from StandaloneMmCore and pool memory + guard management from PiSmmCore. -#Override : 00000002 | StandaloneMmPkg/Core/StandaloneMmCore.inf | 0958e6b72fe8eeaf87f2d0dd61ab4a62 | 2024-07-29T17-47-37 | 5bfab09d1f243366d256ed254ded0413d9b1440d -#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 065a21955cc57cf1c06dabf3d238f34a | 2024-07-29T17-59-14 | 5bfab09d1f243366d256ed254ded0413d9b1440d +#Override : 00000002 | StandaloneMmPkg/Core/StandaloneMmCore.inf | 22aa8d1b884e477e1e078b485974dc90 | 2024-08-28T16-51-15 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c +#Override : 00000002 | MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | b775d90b4549dbd378f23de48a0f3a16 | 2024-08-28T16-42-21 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c [Defines] INF_VERSION = 0x0001001A diff --git a/MmSupervisorPkg/Library/BaseLibSysCall/BaseLib.inf b/MmSupervisorPkg/Library/BaseLibSysCall/BaseLib.inf index f5298356..65a50195 100644 --- a/MmSupervisorPkg/Library/BaseLibSysCall/BaseLib.inf +++ b/MmSupervisorPkg/Library/BaseLibSysCall/BaseLib.inf @@ -12,7 +12,7 @@ # ## -#Override : 00000002 | MdePkg/Library/BaseLib/BaseLib.inf | a1be026b09ddd1811dc0a858afd44d42 | 2024-07-29T18-23-19 | 5bfab09d1f243366d256ed254ded0413d9b1440d +#Override : 00000002 | MdePkg/Library/BaseLib/BaseLib.inf | 956fa1296bf5d3f413da3ebc633d479f | 2024-08-28T16-28-06 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c [Defines] INF_VERSION = 0x00010005 diff --git a/MmSupervisorPkg/Library/BaseLibSysCall/String.c b/MmSupervisorPkg/Library/BaseLibSysCall/String.c index 8b962c25..e78554d0 100644 --- a/MmSupervisorPkg/Library/BaseLibSysCall/String.c +++ b/MmSupervisorPkg/Library/BaseLibSysCall/String.c @@ -407,9 +407,13 @@ StrDecimalToUintn ( ) { UINTN Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (StrDecimalToUintnS (String, (CHAR16 **)NULL, &Result))) { - return MAX_UINTN; + Status = StrDecimalToUintnS (String, (CHAR16 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -456,9 +460,13 @@ StrDecimalToUint64 ( ) { UINT64 Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (StrDecimalToUint64S (String, (CHAR16 **)NULL, &Result))) { - return MAX_UINT64; + Status = StrDecimalToUint64S (String, (CHAR16 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -506,9 +514,13 @@ StrHexToUintn ( ) { UINTN Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (StrHexToUintnS (String, (CHAR16 **)NULL, &Result))) { - return MAX_UINTN; + Status = StrHexToUintnS (String, (CHAR16 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -556,9 +568,13 @@ StrHexToUint64 ( ) { UINT64 Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (StrHexToUint64S (String, (CHAR16 **)NULL, &Result))) { - return MAX_UINT64; + Status = StrHexToUint64S (String, (CHAR16 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -1000,9 +1016,13 @@ AsciiStrDecimalToUintn ( ) { UINTN Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (AsciiStrDecimalToUintnS (String, (CHAR8 **)NULL, &Result))) { - return MAX_UINTN; + Status = AsciiStrDecimalToUintnS (String, (CHAR8 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -1045,9 +1065,13 @@ AsciiStrDecimalToUint64 ( ) { UINT64 Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (AsciiStrDecimalToUint64S (String, (CHAR8 **)NULL, &Result))) { - return MAX_UINT64; + Status = AsciiStrDecimalToUint64S (String, (CHAR8 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -1094,9 +1118,13 @@ AsciiStrHexToUintn ( ) { UINTN Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (AsciiStrHexToUintnS (String, (CHAR8 **)NULL, &Result))) { - return MAX_UINTN; + Status = AsciiStrHexToUintnS (String, (CHAR8 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; @@ -1143,9 +1171,13 @@ AsciiStrHexToUint64 ( ) { UINT64 Result; + // MU_CHANGE Start - CodeQL Change + RETURN_STATUS Status; - if (RETURN_ERROR (AsciiStrHexToUint64S (String, (CHAR8 **)NULL, &Result))) { - return MAX_UINT64; + Status = AsciiStrHexToUint64S (String, (CHAR8 **)NULL, &Result); + if (Status == RETURN_INVALID_PARAMETER) { + Result = 0; + // MU_CHANGE End - CodeQL Change } return Result; diff --git a/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.c b/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.c index 763da8ad..36639765 100644 --- a/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.c +++ b/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.c @@ -618,12 +618,13 @@ DumpSmiHandler ( Print (L">\n"); ImageStruct = GetImageFromRef ((UINTN)SmiHandlerStruct->ImageRef); - if (ImageStruct == NULL) { - ASSERT (ImageStruct != NULL); - return; + // MU_CHANGE - CodeQl Changes - If ImageStruct returned NULL, initialize NameString to an empty string + if (ImageStruct != NULL) { + NameString = GetDriverNameString (ImageStruct); + } else { + NameString = "\0"; } - NameString = GetDriverNameString (ImageStruct); Print (L" \n", SmiHandlerStruct->ImageRef, NameString); if ((ImageStruct != NULL) && (ImageStruct->PdbStringOffset != 0)) { Print (L" %a\n", (UINT8 *)ImageStruct + ImageStruct->PdbStringOffset); diff --git a/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.inf b/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.inf index 351c1ed3..ac39fd2e 100644 --- a/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.inf +++ b/MmSupervisorPkg/Test/MmiHandlerProfileInfo/MmiHandlerProfileInfo.inf @@ -10,7 +10,7 @@ # ## -#Override : 00000002 | MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileAuditTestApp.inf | 312e58d82d8dff03e4133b1484cb8674 | 2024-08-08T06-05-16 | 63fc132a6964231069fb626f25972a5bd0d3fcdc +#Override : 00000002 | MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileAuditTestApp.inf | a67356f0d655f2b04c2f196a9dcf4516 | 2024-08-28T17-13-15 | 0a17aa9da5ebde81bd5e2053ce4df5ff9dedf45c [Defines] INF_VERSION = 0x00010005