Re: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables

["Michael Kubacki" <mikuback@linux.microsoft.com>]

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 <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 <eric.dong@intel.com>; Erich McMillan <emcmillan@microsoft.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Michael Kubacki <mikuback@linux.microsoft.com>; Kumar, Rahul R
>> <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>
>> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Fixes CodeQL alerts for CWE-457:
>> https://cwe.mitre.org/data/definitions/457.html
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Erich McMillan <emcmillan@microsoft.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>> ---
>>   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]
>> -=-=-=-=-=-=
>>
> 
> 
> 
> 
> 
>