[PATCH v1 05/12] MdeModulePkg: Fix conditionally uninitialized variables

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

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

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

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c                        |  5 +--
 MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c                           | 24 ++++++++------
 MdeModulePkg/Core/Dxe/Mem/Page.c                              | 17 +++++-----
 MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c | 25 +++++++++------
 MdeModulePkg/Library/FileExplorerLib/FileExplorer.c           |  5 ++-
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c                      | 33 +++++++++++---------
 MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c      | 11 ++++---
 MdeModulePkg/Universal/HiiDatabaseDxe/Font.c                  | 14 ++++++---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c         |  2 +-
 9 files changed, 80 insertions(+), 56 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 843815d0cb18..14bed5472958 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1407,6 +1407,7 @@ SupportPaletteSnoopAttributes (
   IN EFI_PCI_IO_PROTOCOL_ATTRIBUTE_OPERATION  Operation
   )
 {
+  EFI_STATUS     Status;
   PCI_IO_DEVICE  *Temp;
   UINT16         VGACommand;
 
@@ -1444,13 +1445,13 @@ SupportPaletteSnoopAttributes (
   // Check if they are on the same bus
   //
   if (Temp->Parent == PciIoDevice->Parent) {
-    PCI_READ_COMMAND_REGISTER (Temp, &VGACommand);
+    Status = PCI_READ_COMMAND_REGISTER (Temp, &VGACommand);
 
     //
     // If they are on the same bus, either one can
     // be set to snoop, the other set to decode
     //
-    if ((VGACommand & EFI_PCI_COMMAND_VGA_PALETTE_SNOOP) != 0) {
+    if (!EFI_ERROR (Status) && ((VGACommand & EFI_PCI_COMMAND_VGA_PALETTE_SNOOP) != 0)) {
       //
       // VGA has set to snoop, so GFX can be only set to disable snoop
       //
diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c b/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c
index 48741085e507..496ffbd5c4cc 100644
--- a/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c
+++ b/MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c
@@ -730,10 +730,12 @@ Uhci2ControlTransfer (
 
   Uhc->PciIo->Flush (Uhc->PciIo);
 
-  *TransferResult = QhResult.Result;
+  if (!EFI_ERROR (Status)) {
+    *TransferResult = QhResult.Result;
 
-  if (DataLength != NULL) {
-    *DataLength = QhResult.Complete;
+    if (DataLength != NULL) {
+      *DataLength = QhResult.Complete;
+    }
   }
 
   UhciDestoryTds (Uhc, TDs);
@@ -884,9 +886,11 @@ Uhci2BulkTransfer (
 
   Uhc->PciIo->Flush (Uhc->PciIo);
 
-  *TransferResult = QhResult.Result;
-  *DataToggle     = QhResult.NextToggle;
-  *DataLength     = QhResult.Complete;
+  if (!EFI_ERROR (Status)) {
+    *TransferResult = QhResult.Result;
+    *DataToggle     = QhResult.NextToggle;
+    *DataLength     = QhResult.Complete;
+  }
 
   UhciDestoryTds (Uhc, TDs);
   Uhc->PciIo->Unmap (Uhc->PciIo, DataMap);
@@ -1210,9 +1214,11 @@ Uhci2SyncInterruptTransfer (
   UhciUnlinkTdFromQh (Uhc->SyncIntQh, TDs);
   Uhc->PciIo->Flush (Uhc->PciIo);
 
-  *TransferResult = QhResult.Result;
-  *DataToggle     = QhResult.NextToggle;
-  *DataLength     = QhResult.Complete;
+  if (!EFI_ERROR (Status)) {
+    *TransferResult = QhResult.Result;
+    *DataToggle     = QhResult.NextToggle;
+    *DataLength     = QhResult.Complete;
+  }
 
   UhciDestoryTds (Uhc, TDs);
   Uhc->PciIo->Unmap (Uhc->PciIo, DataMap);
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 160289c1f9ec..2eb07b56b420 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -449,14 +449,15 @@ PromoteMemoryResource (
     //
     Promoted = PromoteGuardedFreePages (&StartAddress, &EndAddress);
     if (Promoted) {
-      CoreGetMemorySpaceDescriptor (StartAddress, &Descriptor);
-      CoreAddRange (
-        EfiConventionalMemory,
-        StartAddress,
-        EndAddress,
-        Descriptor.Capabilities & ~(EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED |
-                                    EFI_MEMORY_TESTED | EFI_MEMORY_RUNTIME)
-        );
+      if (!EFI_ERROR (CoreGetMemorySpaceDescriptor (StartAddress, &Descriptor))) {
+        CoreAddRange (
+          EfiConventionalMemory,
+          StartAddress,
+          EndAddress,
+          Descriptor.Capabilities & ~(EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED |
+                                      EFI_MEMORY_TESTED | EFI_MEMORY_RUNTIME)
+          );
+      }
     }
   }
 
diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c
index cdaa2db15365..e22aaf3039f1 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c
@@ -909,23 +909,28 @@ BootFromFile (
   IN EFI_DEVICE_PATH_PROTOCOL  *FilePath
   )
 {
+  EFI_STATUS                    Status;
   EFI_BOOT_MANAGER_LOAD_OPTION  BootOption;
   CHAR16                        *FileName;
 
+  Status   = EFI_NOT_STARTED;
   FileName = NULL;
 
   FileName = ExtractFileNameFromDevicePath (FilePath);
   if (FileName != NULL) {
-    EfiBootManagerInitializeLoadOption (
-      &BootOption,
-      0,
-      LoadOptionTypeBoot,
-      LOAD_OPTION_ACTIVE,
-      FileName,
-      FilePath,
-      NULL,
-      0
-      );
+    Status = EfiBootManagerInitializeLoadOption (
+               &BootOption,
+               0,
+               LoadOptionTypeBoot,
+               LOAD_OPTION_ACTIVE,
+               FileName,
+               FilePath,
+               NULL,
+               0
+               );
+  }
+
+  if (!EFI_ERROR (Status)) {
     //
     // Since current no boot from removable media directly is allowed */
     //
diff --git a/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c b/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c
index ef949267fcc2..804a03d868f2 100644
--- a/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c
+++ b/MdeModulePkg/Library/FileExplorerLib/FileExplorer.c
@@ -1075,7 +1075,10 @@ LibCreateNewFile (
   NewHandle    = NULL;
   FullFileName = NULL;
 
-  LibGetFileHandleFromDevicePath (gFileExplorerPrivate.RetDevicePath, &FileHandle, &ParentName, &DeviceHandle);
+  if (EFI_ERROR (LibGetFileHandleFromDevicePath (gFileExplorerPrivate.RetDevicePath, &FileHandle, &ParentName, &DeviceHandle))) {
+    return EFI_DEVICE_ERROR;
+  }
+
   FullFileName = LibAppendFileName (ParentName, FileName);
   if (FullFileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 766dde3aaeeb..72de8d3211b7 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -691,6 +691,7 @@ BdsEntry (
   EFI_DEVICE_PATH_PROTOCOL        *FilePath;
   EFI_STATUS                      BootManagerMenuStatus;
   EFI_BOOT_MANAGER_LOAD_OPTION    PlatformDefaultBootOption;
+  BOOLEAN                         PlatformDefaultBootOptionValid;
 
   HotkeyTriggered = NULL;
   Status          = EFI_SUCCESS;
@@ -809,24 +810,24 @@ BdsEntry (
     CpuDeadLoop ();
   }
 
-  Status = EfiBootManagerInitializeLoadOption (
-             &PlatformDefaultBootOption,
-             LoadOptionNumberUnassigned,
-             LoadOptionTypePlatformRecovery,
-             LOAD_OPTION_ACTIVE,
-             L"Default PlatformRecovery",
-             FilePath,
-             NULL,
-             0
-             );
-  ASSERT_EFI_ERROR (Status);
+  PlatformDefaultBootOptionValid = EfiBootManagerInitializeLoadOption (
+                                     &PlatformDefaultBootOption,
+                                     LoadOptionNumberUnassigned,
+                                     LoadOptionTypePlatformRecovery,
+                                     LOAD_OPTION_ACTIVE,
+                                     L"Default PlatformRecovery",
+                                     FilePath,
+                                     NULL,
+                                     0
+                                     ) == EFI_SUCCESS;
+  ASSERT (PlatformDefaultBootOptionValid == TRUE);
 
   //
   // System firmware must include a PlatformRecovery#### variable specifying
   // a short-form File Path Media Device Path containing the platform default
   // file path for removable media if the platform supports Platform Recovery.
   //
-  if (PcdGetBool (PcdPlatformRecoverySupport)) {
+  if (PlatformDefaultBootOptionValid && PcdGetBool (PcdPlatformRecoverySupport)) {
     LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery);
     if (EfiBootManagerFindLoadOption (&PlatformDefaultBootOption, LoadOptions, LoadOptionCount) == -1) {
       for (Index = 0; Index < LoadOptionCount; Index++) {
@@ -1104,15 +1105,17 @@ BdsEntry (
       LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery);
       ProcessLoadOptions (LoadOptions, LoadOptionCount);
       EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
-    } else {
+    } else if (PlatformDefaultBootOptionValid) {
       //
       // When platform recovery is not enabled, still boot to platform default file path.
       //
-      EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption);
+      PlatformDefaultBootOptionValid = EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption) == EFI_SUCCESS;
     }
   }
 
-  EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption);
+  if (PlatformDefaultBootOptionValid) {
+    EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption);
+  }
 
   DEBUG ((DEBUG_ERROR, "[Bds] Unable to boot!\n"));
   PlatformBootManagerUnableToBoot ();
diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
index dca3c1df07ba..0d4cfa4cf06f 100644
--- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
@@ -944,13 +944,14 @@ PrintMismatchMenuInfo (
   UINTN                          FormsetBufferSize;
 
   Question = MenuOption->ThisTag;
-  HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, &FormsetBuffer, &FormsetBufferSize);
 
-  FormSetTitleStr = GetToken (FormsetBuffer->FormSetTitle, gFormData->HiiHandle);
-  FormTitleStr    = GetToken (gFormData->FormTitle, gFormData->HiiHandle);
+  if (!EFI_ERROR (HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, &FormsetBuffer, &FormsetBufferSize))) {
+    FormSetTitleStr = GetToken (FormsetBuffer->FormSetTitle, gFormData->HiiHandle);
+    FormTitleStr    = GetToken (gFormData->FormTitle, gFormData->HiiHandle);
 
-  DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset    : Formset Guid = %g,  FormSet title = %s\n", gEfiCallerBaseName, &gFormData->FormSetGuid, FormSetTitleStr));
-  DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form       : FormId = %d,  Form title = %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr));
+    DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset    : Formset Guid = %g,  FormSet title = %s\n", gEfiCallerBaseName, &gFormData->FormSetGuid, FormSetTitleStr));
+    DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form       : FormId = %d,  Form title = %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr));
+  }
 
   if (Question->OpCode->OpCode == EFI_IFR_ORDERED_LIST_OP) {
     QuestionName = GetToken (((EFI_IFR_ORDERED_LIST *)MenuOption->ThisTag->OpCode)->Question.Header.Prompt, gFormData->HiiHandle);
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
index 399f90feb783..8a0b12f72fbe 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
@@ -1745,6 +1745,7 @@ HiiStringToImage (
   Attributes = (UINT8 *)AllocateZeroPool (StrLength * sizeof (UINT8));
   ASSERT (Attributes != NULL);
 
+  FontInfo      = NULL;
   RowInfo       = NULL;
   Status        = EFI_SUCCESS;
   StringIn2     = NULL;
@@ -1787,11 +1788,14 @@ HiiStringToImage (
       Background  = ((EFI_FONT_DISPLAY_INFO *)StringInfo)->BackgroundColor;
     } else if (Status == EFI_SUCCESS) {
       FontInfo = &StringInfoOut->FontInfo;
-      IsFontInfoExisted (Private, FontInfo, NULL, NULL, &GlobalFont);
-      Height     = GlobalFont->FontPackage->Height;
-      BaseLine   = GlobalFont->FontPackage->BaseLine;
-      Foreground = StringInfoOut->ForegroundColor;
-      Background = StringInfoOut->BackgroundColor;
+      if (IsFontInfoExisted (Private, FontInfo, NULL, NULL, &GlobalFont)) {
+        Height     = GlobalFont->FontPackage->Height;
+        BaseLine   = GlobalFont->FontPackage->BaseLine;
+        Foreground = StringInfoOut->ForegroundColor;
+        Background = StringInfoOut->BackgroundColor;
+      } else {
+        goto Exit;
+      }
     } else {
       goto Exit;
     }
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6c1a3440ac8c..b64fcbdc7281 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2453,7 +2453,7 @@ VariableServiceGetVariable (
   AcquireLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);
 
   Status = FindVariable (VariableName, VendorGuid, &Variable, &mVariableModuleGlobal->VariableGlobal, FALSE);
-  if ((Variable.CurrPtr == NULL) || EFI_ERROR (Status)) {
+  if (EFI_ERROR (Status) || (Variable.CurrPtr == NULL)) {
     goto Done;
   }
 
-- 
2.28.0.windows.1