I spoke to Erich offline and he mentioned that a previous coding practice he used specified unsigned integer literals when intended so he applied that here.

Thanks,
Michael

On 3/10/2023 5:59 PM, Michael Kubacki wrote:
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.d...@intel.com>; Erich McMillan <emcmil...@microsoft.com>; Kinney, Michael D <michael.d.kin...@intel.com>; Michael Kubacki <mikub...@linux.microsoft.com>; Kumar, Rahul R
<rahul.r.ku...@intel.com>; Ni, Ray <ray...@intel.com>
Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables

From: Michael Kubacki <michael.kuba...@microsoft.com>

Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Eric Dong <eric.d...@intel.com>
Cc: Erich McMillan <emcmil...@microsoft.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Michael Kubacki <mikub...@linux.microsoft.com>
Cc: Rahul Kumar <rahul1.ku...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Co-authored-by: Erich McMillan <emcmil...@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kuba...@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+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kin...@intel.com]
-=-=-=-=-=-=









-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101493): https://edk2.groups.io/g/devel/message/101493
Mute This Topic: https://groups.io/mt/97526805/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to