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.web10.138.1679673041308173634 for ; Fri, 24 Mar 2023 08:50:41 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=eYyqk1eG; 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 2E0B520FC447; Fri, 24 Mar 2023 08:50:40 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 2E0B520FC447 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1679673040; bh=boq95BUF2jglOjMjic8Z0R0sUv/9C3aSQ+oxl1nupR8=; h=Date:Subject:From:To:Cc:References:In-Reply-To:From; b=eYyqk1eGfyplIQu/yNab2tj1Scp8d0YkK+uYKZeKNka6eGSb+LqimEyGBNAPS/S4F FWtMUm1sEDGB/XIZJCZJeZ4C3oqspnO/RIE05dNL6TEAvyhCgnlU6L8RGoUipLKEas 6y+VFsRdo/wduLrhT2wxd7wmLk2j+r7x8fa/vWf8= Message-ID: Date: Fri, 24 Mar 2023 11:50:39 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables From: "Michael Kubacki" 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> <01b6726a-0011-250e-ccbf-0c0ed2e4c654@linux.microsoft.com> In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hi Mike, Would you like a change here? Thanks, Michael On 3/21/2023 10:25 AM, Michael Kubacki wrote: > I spoke to Erich offline and he mentioned that a previous coding=20 > practice he used specified unsigned integer literals when intended so he= =20 > applied that here. >=20 > Thanks, > Michael >=20 > On 3/10/2023 5:59 PM, Michael Kubacki wrote: >> Erich introduced this particular change, he may be able to provide=20 >> more info. >> >> On 3/10/2023 3:03 PM, Michael D Kinney wrote: >>> >>> >>>> -----Original Message----- >>>> From: devel@edk2.groups.io On Behalf Of=20 >>>> Michael Kubacki >>>> Sent: Friday, March 10, 2023 10:43 AM >>>> To: devel@edk2.groups.io >>>> Cc: Dong, Eric ; Erich McMillan=20 >>>> ; Kinney, Michael D >>>> ; Michael Kubacki=20 >>>> ; Kumar, Rahul R >>>> ; Ni, Ray >>>> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally= =20 >>>> 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 >>>> --- >>>> =C2=A0 UefiCpuPkg/CpuMpPei/CpuBist.c=C2=A0=C2=A0 | 8 +++++++- >>>> =C2=A0 UefiCpuPkg/CpuMpPei/CpuMpPei.c=C2=A0 | 8 +++++++- >>>> =C2=A0 UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 ++++++++- >>>> =C2=A0 3 files changed, 22 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuBist.c=20 >>>> b/UefiCpuPkg/CpuMpPei/CpuBist.c >>>> index 7dc93cd784d4..122808139b87 100644 >>>> --- a/UefiCpuPkg/CpuMpPei/CpuBist.c >>>> +++ b/UefiCpuPkg/CpuMpPei/CpuBist.c >>>> @@ -175,7 +175,13 @@ CollectBistDataFromPpi ( >>>> =C2=A0=C2=A0=C2=A0 EFI_SEC_PLATFORM_INFORMATION_RECORD2=C2=A0 *Platfor= mInformationRecord2; >>>> =C2=A0=C2=A0=C2=A0 EFI_SEC_PLATFORM_INFORMATION_CPU=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 *CpuInstanceInHob; >>>> >>>> -=C2=A0 MpInitLibGetNumberOfProcessors (&NumberOfProcessors,=20 >>>> &NumberOfEnabledProcessors); >>>> +=C2=A0 Status =3D MpInitLibGetNumberOfProcessors (&NumberOfProcessors= ,=20 >>>> &NumberOfEnabledProcessors); >>>> +=C2=A0 ASSERT_EFI_ERROR (Status); >>>> + >>>> +=C2=A0 if (EFI_ERROR (Status)) { >>>> +=C2=A0=C2=A0=C2=A0 NumberOfProcessors=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 =3D 1u; >>>> +=C2=A0=C2=A0=C2=A0 NumberOfEnabledProcessors =3D 1u; >>> >>> Why is '1u' used instead of '1'?=C2=A0 I don't see a lot of usage of th= e=20 >>> postfix >>> unless the const needs to be forced to a larger bitwidth than default= =20 >>> int type. >>> >>>> +=C2=A0 } >>>> >>>> =C2=A0=C2=A0=C2=A0 BistInformationSize =3D sizeof=20 >>>> (EFI_SEC_PLATFORM_INFORMATION_RECORD2) + >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 sizeof (EFI_SEC_PLATFORM_INFORMATION_CPU)=20 >>>> * NumberOfProcessors; >>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c=20 >>>> b/UefiCpuPkg/CpuMpPei/CpuMpPei.c >>>> index e7f1fe9f426c..a84304273168 100644 >>>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c >>>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c >>>> @@ -473,7 +473,13 @@ InitializeMpExceptionStackSwitchHandlers ( >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return; >>>> =C2=A0=C2=A0=C2=A0 } >>>> >>>> -=C2=A0 MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); >>>> +=C2=A0 Status =3D MpInitLibGetNumberOfProcessors (&NumberOfProcessors= , NULL); >>>> +=C2=A0 ASSERT_EFI_ERROR (Status); >>>> + >>>> +=C2=A0 if (EFI_ERROR (Status)) { >>>> +=C2=A0=C2=A0=C2=A0 NumberOfProcessors =3D 1u; >>>> +=C2=A0 } >>>> + >>>> =C2=A0=C2=A0=C2=A0 SwitchStackData =3D AllocatePages (EFI_SIZE_TO_PAGE= S=20 >>>> (NumberOfProcessors * sizeof >>>> (EXCEPTION_STACK_SWITCH_CONTEXT))); >>>> =C2=A0=C2=A0=C2=A0 ASSERT (SwitchStackData !=3D NULL); >>>> =C2=A0=C2=A0=C2=A0 ZeroMem (SwitchStackData, NumberOfProcessors * size= of=20 >>>> (EXCEPTION_STACK_SWITCH_CONTEXT)); >>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c=20 >>>> b/UefiCpuPkg/CpuMpPei/CpuPaging.c >>>> index 135422225340..1322fcb77f28 100644 >>>> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c >>>> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c >>>> @@ -538,6 +538,7 @@ SetupStackGuardPage ( >>>> =C2=A0=C2=A0=C2=A0 UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 NumberOfProcessors; >>>> =C2=A0=C2=A0=C2=A0 UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Bsp; >>>> =C2=A0=C2=A0=C2=A0 UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Index; >>>> +=C2=A0 EFI_STATUS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 Status; >>>> >>>> =C2=A0=C2=A0=C2=A0 // >>>> =C2=A0=C2=A0=C2=A0 // One extra page at the bottom of the stack is nee= ded for Guard=20 >>>> page. >>>> @@ -547,7 +548,13 @@ SetupStackGuardPage ( >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ASSERT (FALSE); >>>> =C2=A0=C2=A0=C2=A0 } >>>> >>>> -=C2=A0 MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); >>>> +=C2=A0 Status =3D MpInitLibGetNumberOfProcessors (&NumberOfProcessors= , NULL); >>>> +=C2=A0 ASSERT_EFI_ERROR (Status); >>>> + >>>> +=C2=A0 if (EFI_ERROR (Status)) { >>>> +=C2=A0=C2=A0=C2=A0 NumberOfProcessors =3D 1u; >>>> +=C2=A0 } >>>> + >>>> =C2=A0=C2=A0=C2=A0 MpInitLibWhoAmI (&Bsp); >>>> =C2=A0=C2=A0=C2=A0 for (Index =3D 0; Index < NumberOfProcessors; ++Ind= ex) { >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 StackBase =3D 0; >>>> --=20 >>>> 2.39.2.windows.1 >>>> >>>> >>>> >>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>> Groups.io Links: You receive all messages sent to this group. >>>> View/Reply Online (#101030):=20 >>>> 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=20 >>>> [michael.d.kinney@intel.com] >>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>> >>> >>> >>> >>>=20 >>> >>>