[PATCH] UefiCpuPkg RegisterCpuFeaturesLib: NumberOfCpus may be uninitialized

[PATCH] UefiCpuPkg RegisterCpuFeaturesLib: NumberOfCpus may be uninitialized

[Zeng, Star]

NumberOfCpus local variable in GetAcpiCpuData will be uninitialized
when CpuS3DataDxe runs before DxeRegisterCpuFeaturesLib (linked by
CpuFeaturesDxe) because there is no code to initialize it at
(AcpiCpuData != NULL) execution path.

The issue is exposed after cefad282fb31aff3e1a6dcbd368cbbffc3fce900
and 38ee7bafa72f58982f99ac6f61eef160f80bad69.
There was negligence in that code review.
One further topic may be "Could EDK2 CI be enhanced to catch this kind
of uninitialized local variable case?". :)

This patch fixes this regression issue.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c      | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 7bb92404027f..60daa5cc87f0 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -957,6 +957,7 @@ GetAcpiCpuData (
     //
     // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
     //
+    NumberOfCpus = AcpiCpuData->NumberOfCpus;
     TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
     RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
     ASSERT (RegisterTable != NULL);
-- 
2.27.0.windows.1

Re: [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: NumberOfCpus may be uninitialized

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Zeng, Star <star.zeng@intel.com>
> Sent: Thursday, January 21, 2021 5:38 PM
> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: NumberOfCpus may be
> uninitialized
> 
> NumberOfCpus local variable in GetAcpiCpuData will be uninitialized
> when CpuS3DataDxe runs before DxeRegisterCpuFeaturesLib (linked by
> CpuFeaturesDxe) because there is no code to initialize it at
> (AcpiCpuData != NULL) execution path.
> 
> The issue is exposed after cefad282fb31aff3e1a6dcbd368cbbffc3fce900
> and 38ee7bafa72f58982f99ac6f61eef160f80bad69.
> There was negligence in that code review.
> One further topic may be "Could EDK2 CI be enhanced to catch this kind
> of uninitialized local variable case?". :)
> 
> This patch fixes this regression issue.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c      | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> index 7bb92404027f..60daa5cc87f0 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -957,6 +957,7 @@ GetAcpiCpuData (
>      //
>      // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for
> all CPUs
>      //
> +    NumberOfCpus = AcpiCpuData->NumberOfCpus;
>      TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
>      RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
>      ASSERT (RegisterTable != NULL);
> --
> 2.27.0.windows.1