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


Michael Kubacki
 

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

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

Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Cc: Erich McMillan <emcmillan@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Liming Gao <gaoliming@...>
---
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/Pc=
i/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;
=20
@@ -1444,13 +1445,13 @@ SupportPaletteSnoopAttributes (
// Check if they are on the same bus
//
if (Temp->Parent =3D=3D PciIoDevice->Parent) {
- PCI_READ_COMMAND_REGISTER (Temp, &VGACommand);
+ Status =3D PCI_READ_COMMAND_REGISTER (Temp, &VGACommand);
=20
//
// 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) !=3D 0) {
+ if (!EFI_ERROR (Status) && ((VGACommand & EFI_PCI_COMMAND_VGA_PALETT=
E_SNOOP) !=3D 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/U=
hciDxe/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 (
=20
Uhc->PciIo->Flush (Uhc->PciIo);
=20
- *TransferResult =3D QhResult.Result;
+ if (!EFI_ERROR (Status)) {
+ *TransferResult =3D QhResult.Result;
=20
- if (DataLength !=3D NULL) {
- *DataLength =3D QhResult.Complete;
+ if (DataLength !=3D NULL) {
+ *DataLength =3D QhResult.Complete;
+ }
}
=20
UhciDestoryTds (Uhc, TDs);
@@ -884,9 +886,11 @@ Uhci2BulkTransfer (
=20
Uhc->PciIo->Flush (Uhc->PciIo);
=20
- *TransferResult =3D QhResult.Result;
- *DataToggle =3D QhResult.NextToggle;
- *DataLength =3D QhResult.Complete;
+ if (!EFI_ERROR (Status)) {
+ *TransferResult =3D QhResult.Result;
+ *DataToggle =3D QhResult.NextToggle;
+ *DataLength =3D QhResult.Complete;
+ }
=20
UhciDestoryTds (Uhc, TDs);
Uhc->PciIo->Unmap (Uhc->PciIo, DataMap);
@@ -1210,9 +1214,11 @@ Uhci2SyncInterruptTransfer (
UhciUnlinkTdFromQh (Uhc->SyncIntQh, TDs);
Uhc->PciIo->Flush (Uhc->PciIo);
=20
- *TransferResult =3D QhResult.Result;
- *DataToggle =3D QhResult.NextToggle;
- *DataLength =3D QhResult.Complete;
+ if (!EFI_ERROR (Status)) {
+ *TransferResult =3D QhResult.Result;
+ *DataToggle =3D QhResult.NextToggle;
+ *DataLength =3D QhResult.Complete;
+ }
=20
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 =3D PromoteGuardedFreePages (&StartAddress, &EndAddress);
if (Promoted) {
- CoreGetMemorySpaceDescriptor (StartAddress, &Descriptor);
- CoreAddRange (
- EfiConventionalMemory,
- StartAddress,
- EndAddress,
- Descriptor.Capabilities & ~(EFI_MEMORY_PRESENT | EFI_MEMORY_INIT=
IALIZED |
- EFI_MEMORY_TESTED | EFI_MEMORY_RUNTI=
ME)
- );
+ if (!EFI_ERROR (CoreGetMemorySpaceDescriptor (StartAddress, &Descr=
iptor))) {
+ CoreAddRange (
+ EfiConventionalMemory,
+ StartAddress,
+ EndAddress,
+ Descriptor.Capabilities & ~(EFI_MEMORY_PRESENT | EFI_MEMORY_IN=
ITIALIZED |
+ EFI_MEMORY_TESTED | EFI_MEMORY_RUN=
TIME)
+ );
+ }
}
}
=20
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;
=20
+ Status =3D EFI_NOT_STARTED;
FileName =3D NULL;
=20
FileName =3D ExtractFileNameFromDevicePath (FilePath);
if (FileName !=3D NULL) {
- EfiBootManagerInitializeLoadOption (
- &BootOption,
- 0,
- LoadOptionTypeBoot,
- LOAD_OPTION_ACTIVE,
- FileName,
- FilePath,
- NULL,
- 0
- );
+ Status =3D 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/MdeMod=
ulePkg/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 =3D NULL;
FullFileName =3D NULL;
=20
- LibGetFileHandleFromDevicePath (gFileExplorerPrivate.RetDevicePath, &F=
ileHandle, &ParentName, &DeviceHandle);
+ if (EFI_ERROR (LibGetFileHandleFromDevicePath (gFileExplorerPrivate.Re=
tDevicePath, &FileHandle, &ParentName, &DeviceHandle))) {
+ return EFI_DEVICE_ERROR;
+ }
+
FullFileName =3D LibAppendFileName (ParentName, FileName);
if (FullFileName =3D=3D NULL) {
return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Univ=
ersal/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;
=20
HotkeyTriggered =3D NULL;
Status =3D EFI_SUCCESS;
@@ -809,24 +810,24 @@ BdsEntry (
CpuDeadLoop ();
}
=20
- Status =3D EfiBootManagerInitializeLoadOption (
- &PlatformDefaultBootOption,
- LoadOptionNumberUnassigned,
- LoadOptionTypePlatformRecovery,
- LOAD_OPTION_ACTIVE,
- L"Default PlatformRecovery",
- FilePath,
- NULL,
- 0
- );
- ASSERT_EFI_ERROR (Status);
+ PlatformDefaultBootOptionValid =3D EfiBootManagerInitializeLoadOption =
(
+ &PlatformDefaultBootOption,
+ LoadOptionNumberUnassigned,
+ LoadOptionTypePlatformRecovery,
+ LOAD_OPTION_ACTIVE,
+ L"Default PlatformRecovery",
+ FilePath,
+ NULL,
+ 0
+ ) =3D=3D EFI_SUCCESS;
+ ASSERT (PlatformDefaultBootOptionValid =3D=3D TRUE);
=20
//
// System firmware must include a PlatformRecovery#### variable specif=
ying
// a short-form File Path Media Device Path containing the platform de=
fault
// file path for removable media if the platform supports Platform Rec=
overy.
//
- if (PcdGetBool (PcdPlatformRecoverySupport)) {
+ if (PlatformDefaultBootOptionValid && PcdGetBool (PcdPlatformRecoveryS=
upport)) {
LoadOptions =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, Load=
OptionTypePlatformRecovery);
if (EfiBootManagerFindLoadOption (&PlatformDefaultBootOption, LoadOp=
tions, LoadOptionCount) =3D=3D -1) {
for (Index =3D 0; Index < LoadOptionCount; Index++) {
@@ -1104,15 +1105,17 @@ BdsEntry (
LoadOptions =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, Lo=
adOptionTypePlatformRecovery);
ProcessLoadOptions (LoadOptions, LoadOptionCount);
EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
- } else {
+ } else if (PlatformDefaultBootOptionValid) {
//
// When platform recovery is not enabled, still boot to platform d=
efault file path.
//
- EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption);
+ PlatformDefaultBootOptionValid =3D EfiBootManagerProcessLoadOption=
(&PlatformDefaultBootOption) =3D=3D EFI_SUCCESS;
}
}
=20
- EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption);
+ if (PlatformDefaultBootOptionValid) {
+ EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption);
+ }
=20
DEBUG ((DEBUG_ERROR, "[Bds] Unable to boot!\n"));
PlatformBootManagerUnableToBoot ();
diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c b/M=
deModulePkg/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;
=20
Question =3D MenuOption->ThisTag;
- HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, &FormsetBuffer, &For=
msetBufferSize);
=20
- FormSetTitleStr =3D GetToken (FormsetBuffer->FormSetTitle, gFormData->=
HiiHandle);
- FormTitleStr =3D GetToken (gFormData->FormTitle, gFormData->HiiHand=
le);
+ if (!EFI_ERROR (HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, &For=
msetBuffer, &FormsetBufferSize))) {
+ FormSetTitleStr =3D GetToken (FormsetBuffer->FormSetTitle, gFormData=
->HiiHandle);
+ FormTitleStr =3D GetToken (gFormData->FormTitle, gFormData->HiiHa=
ndle);
=20
- DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset : Formset Guid =3D %=
g, FormSet title =3D %s\n", gEfiCallerBaseName, &gFormData->FormSetGuid,=
FormSetTitleStr));
- DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form : FormId =3D %d, Form=
title =3D %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr));
+ DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset : Formset Guid =3D=
%g, FormSet title =3D %s\n", gEfiCallerBaseName, &gFormData->FormSetGui=
d, FormSetTitleStr));
+ DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form : FormId =3D %d, Fo=
rm title =3D %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr)=
);
+ }
=20
if (Question->OpCode->OpCode =3D=3D EFI_IFR_ORDERED_LIST_OP) {
QuestionName =3D GetToken (((EFI_IFR_ORDERED_LIST *)MenuOption->This=
Tag->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 =3D (UINT8 *)AllocateZeroPool (StrLength * sizeof (UINT8));
ASSERT (Attributes !=3D NULL);
=20
+ FontInfo =3D NULL;
RowInfo =3D NULL;
Status =3D EFI_SUCCESS;
StringIn2 =3D NULL;
@@ -1787,11 +1788,14 @@ HiiStringToImage (
Background =3D ((EFI_FONT_DISPLAY_INFO *)StringInfo)->BackgroundC=
olor;
} else if (Status =3D=3D EFI_SUCCESS) {
FontInfo =3D &StringInfoOut->FontInfo;
- IsFontInfoExisted (Private, FontInfo, NULL, NULL, &GlobalFont);
- Height =3D GlobalFont->FontPackage->Height;
- BaseLine =3D GlobalFont->FontPackage->BaseLine;
- Foreground =3D StringInfoOut->ForegroundColor;
- Background =3D StringInfoOut->BackgroundColor;
+ if (IsFontInfoExisted (Private, FontInfo, NULL, NULL, &GlobalFont)=
) {
+ Height =3D GlobalFont->FontPackage->Height;
+ BaseLine =3D GlobalFont->FontPackage->BaseLine;
+ Foreground =3D StringInfoOut->ForegroundColor;
+ Background =3D StringInfoOut->BackgroundColor;
+ } else {
+ goto Exit;
+ }
} else {
goto Exit;
}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeM=
odulePkg/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.Vari=
ableServicesLock);
=20
Status =3D FindVariable (VariableName, VendorGuid, &Variable, &mVariab=
leModuleGlobal->VariableGlobal, FALSE);
- if ((Variable.CurrPtr =3D=3D NULL) || EFI_ERROR (Status)) {
+ if (EFI_ERROR (Status) || (Variable.CurrPtr =3D=3D NULL)) {
goto Done;
}
=20
--=20
2.28.0.windows.1