From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.34061.1678489167884742720 for ; Fri, 10 Mar 2023 14:59:27 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=rts7ctfo; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id AE34420C56BB; Fri, 10 Mar 2023 14:59:26 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AE34420C56BB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1678489167; bh=uoAgW21uMXO1NHIy31GK4fL6J9TeCtmzIAWcqn1vzBs=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=rts7ctfo4sfI5Xokr34APWQYJhciLYNo7G7NcFU1BK8CWDNPWp7LMYavpa3a1JryY +eY6lIiw9x7XJXZJTEbRPBOC8fdZnKRDOid37Nyb80igFrA7W8wuSHcv5R3QELUbNi PUf4zvqrIOTmihRX/KsgZwHOJXe5L88l9Knz6DkM= Message-ID: <01b6726a-0011-250e-ccbf-0c0ed2e4c654@linux.microsoft.com> Date: Fri, 10 Mar 2023 17:59:25 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables To: devel@edk2.groups.io, michael.d.kinney@intel.com, Erich McMillan Cc: "Dong, Eric" , "Kumar, Rahul R" , "Ni, Ray" References: <20230310184238.2999-1-mikuback@linux.microsoft.com> <20230310184238.2999-11-mikuback@linux.microsoft.com> From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Erich introduced this particular change, he may be able to provide more info. On 3/10/2023 3:03 PM, Michael D Kinney wrote: > > >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Michael Kubacki >> Sent: Friday, March 10, 2023 10:43 AM >> To: devel@edk2.groups.io >> Cc: Dong, Eric ; Erich McMillan ; Kinney, Michael D >> ; Michael Kubacki ; Kumar, Rahul R >> ; Ni, Ray >> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables >> >> From: Michael Kubacki >> >> Fixes CodeQL alerts for CWE-457: >> https://cwe.mitre.org/data/definitions/457.html >> >> Cc: Eric Dong >> Cc: Erich McMillan >> Cc: Michael D Kinney >> Cc: Michael Kubacki >> Cc: Rahul Kumar >> Cc: Ray Ni >> Co-authored-by: Erich McMillan >> Signed-off-by: Michael Kubacki >> --- >> UefiCpuPkg/CpuMpPei/CpuBist.c | 8 +++++++- >> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 8 +++++++- >> UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 ++++++++- >> 3 files changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/UefiCpuPkg/CpuMpPei/CpuBist.c b/UefiCpuPkg/CpuMpPei/CpuBist.c >> index 7dc93cd784d4..122808139b87 100644 >> --- a/UefiCpuPkg/CpuMpPei/CpuBist.c >> +++ b/UefiCpuPkg/CpuMpPei/CpuBist.c >> @@ -175,7 +175,13 @@ CollectBistDataFromPpi ( >> EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2; >> EFI_SEC_PLATFORM_INFORMATION_CPU *CpuInstanceInHob; >> >> - MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors); >> + Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors); >> + ASSERT_EFI_ERROR (Status); >> + >> + if (EFI_ERROR (Status)) { >> + NumberOfProcessors = 1u; >> + NumberOfEnabledProcessors = 1u; > > Why is '1u' used instead of '1'? I don't see a lot of usage of the postfix > unless the const needs to be forced to a larger bitwidth than default int type. > >> + } >> >> BistInformationSize = sizeof (EFI_SEC_PLATFORM_INFORMATION_RECORD2) + >> sizeof (EFI_SEC_PLATFORM_INFORMATION_CPU) * NumberOfProcessors; >> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c >> index e7f1fe9f426c..a84304273168 100644 >> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c >> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c >> @@ -473,7 +473,13 @@ InitializeMpExceptionStackSwitchHandlers ( >> return; >> } >> >> - MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); >> + Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); >> + ASSERT_EFI_ERROR (Status); >> + >> + if (EFI_ERROR (Status)) { >> + NumberOfProcessors = 1u; >> + } >> + >> SwitchStackData = AllocatePages (EFI_SIZE_TO_PAGES (NumberOfProcessors * sizeof >> (EXCEPTION_STACK_SWITCH_CONTEXT))); >> ASSERT (SwitchStackData != NULL); >> ZeroMem (SwitchStackData, NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)); >> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c >> index 135422225340..1322fcb77f28 100644 >> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c >> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c >> @@ -538,6 +538,7 @@ SetupStackGuardPage ( >> UINTN NumberOfProcessors; >> UINTN Bsp; >> UINTN Index; >> + EFI_STATUS Status; >> >> // >> // One extra page at the bottom of the stack is needed for Guard page. >> @@ -547,7 +548,13 @@ SetupStackGuardPage ( >> ASSERT (FALSE); >> } >> >> - MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); >> + Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); >> + ASSERT_EFI_ERROR (Status); >> + >> + if (EFI_ERROR (Status)) { >> + NumberOfProcessors = 1u; >> + } >> + >> MpInitLibWhoAmI (&Bsp); >> for (Index = 0; Index < NumberOfProcessors; ++Index) { >> StackBase = 0; >> -- >> 2.39.2.windows.1 >> >> >> >> -=-=-=-=-=-= >> Groups.io Links: You receive all messages sent to this group. >> View/Reply Online (#101030): https://edk2.groups.io/g/devel/message/101030 >> Mute This Topic: https://groups.io/mt/97526805/1643496 >> Group Owner: devel+owner@edk2.groups.io >> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] >> -=-=-=-=-=-= >> > > > > > >