Hi Michael, comments below.
Mike > -----Original Message----- > From: [email protected] <[email protected]> On Behalf Of Michael Kubacki > Sent: Wednesday, November 9, 2022 9:33 AM > To: [email protected] > Cc: Dong, Eric <[email protected]>; Erich McMillan > <[email protected]>; Kinney, Michael D > <[email protected]>; Michael Kubacki <[email protected]>; > Kumar, Rahul R <[email protected]>; > Ni, Ray <[email protected]> > Subject: [edk2-devel] [PATCH v1 10/12] UefiCpuPkg: Fix conditionally > uninitialized variables > > From: Michael Kubacki <[email protected]> > > Fixes CodeQL alerts for CWE-457: > https://cwe.mitre.org/data/definitions/457.html > > Cc: Eric Dong <[email protected]> > Cc: Erich McMillan <[email protected]> > Cc: Michael D Kinney <[email protected]> > Cc: Michael Kubacki <[email protected]> > Cc: Rahul Kumar <[email protected]> > Cc: Ray Ni <[email protected]> > Co-authored-by: Erich McMillan <[email protected]> > Signed-off-by: Michael Kubacki <[email protected]> > --- > 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); I think this ASSERT() should be removed. The added error status check looks correct. > + > + if (EFI_ERROR (Status)) { > + NumberOfProcessors = 1u; > + NumberOfEnabledProcessors = 1u; > + } > > 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); I think this ASSERT() should be removed. The added error status check looks correct. > + > + 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); I think this ASSERT() should be removed. The added error status check looks correct. > + > + if (EFI_ERROR (Status)) { > + NumberOfProcessors = 1u; > + } > + > MpInitLibWhoAmI (&Bsp); > for (Index = 0; Index < NumberOfProcessors; ++Index) { > StackBase = 0; > -- > 2.28.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#96156): https://edk2.groups.io/g/devel/message/96156 > Mute This Topic: https://groups.io/mt/94918104/1643496 > Group Owner: [email protected] > Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] > -=-=-=-=-=-= > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96602): https://edk2.groups.io/g/devel/message/96602 Mute This Topic: https://groups.io/mt/94918104/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
