From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io
Cc: Dandan Bi <dandan.bi@intel.com>, Eric Dong <eric.dong@intel.com>,
Erich McMillan <emcmillan@microsoft.com>,
Guomin Jiang <guomin.jiang@intel.com>,
Jian J Wang <jian.j.wang@intel.com>,
Liming Gao <gaoliming@byosoft.com.cn>,
Michael Kubacki <mikuback@linux.microsoft.com>,
Ray Ni <ray.ni@intel.com>, Zhichao Gao <zhichao.gao@intel.com>
Subject: [PATCH v6 05/12] MdeModulePkg: Fix conditionally uninitialized variables
Date: Fri, 24 Mar 2023 16:48:31 -0400 [thread overview]
Message-ID: <20230324204838.1485-6-mikuback@linux.microsoft.com> (raw)
In-Reply-To: <20230324204838.1485-1-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>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
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 5903ce7ab525..41af50b3d5ab 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 14c176887a55..3eb7d935b4d2 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.40.0.windows.1
next prev parent reply other threads:[~2023-03-24 20:49 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-24 20:48 [PATCH v6 00/12] Enable New CodeQL Queries Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 02/12] BaseTools/PatchCheck.py: Add PCCTS to tab exemption list Michael Kubacki
2023-03-24 20:52 ` [edk2-devel] " Rebecca Cran
[not found] ` <174F7634AC74F535.7603@groups.io>
2023-03-24 20:56 ` Rebecca Cran
2023-03-24 21:27 ` Michael Kubacki
2023-03-24 22:33 ` Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 03/12] BaseTools/VfrCompile: Fix potential buffer overwrites Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 04/12] CryptoPkg: Fix conditionally uninitialized variable Michael Kubacki
2023-03-24 20:48 ` Michael Kubacki [this message]
2023-03-24 20:48 ` [PATCH v6 06/12] MdePkg: Fix conditionally uninitialized variables Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 07/12] NetworkPkg: " Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 08/12] PcAtChipsetPkg: " Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 09/12] ShellPkg: " Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 10/12] UefiCpuPkg: " Michael Kubacki
2023-03-24 21:16 ` Michael D Kinney
2023-03-24 20:48 ` [PATCH v6 11/12] .github/codeql/edk2.qls: Enable CWE 457, 676, and 758 queries Michael Kubacki
2023-03-24 20:48 ` [PATCH v6 12/12] .github/codeql/edk2.qls: Enable CWE 120, 787, and 805 queries Michael Kubacki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230324204838.1485-6-mikuback@linux.microsoft.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox