From 07251f3c6a9aff09eb2778f8d5db51348fca8e18 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Tue, 8 Nov 2022 15:24:54 -0500 Subject: MdeModulePkg: Fix conditionally uninitialized variables Fixes CodeQL alerts for CWE-457: https://cwe.mitre.org/data/definitions/457.html Cc: Dandan Bi Cc: Eric Dong Cc: Erich McMillan Cc: Guomin Jiang Cc: Jian J Wang Cc: Liming Gao Cc: Michael Kubacki Cc: Ray Ni Cc: Zhichao Gao Co-authored-by: Erich McMillan Signed-off-by: Michael Kubacki Reviewed-by: Liming Gao Reviewed-by: Oliver Smith-Denny --- MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 5 ++-- MdeModulePkg/Bus/Pci/UhciDxe/Uhci.c | 24 ++++++++++------ MdeModulePkg/Core/Dxe/Mem/Page.c | 17 +++++------ .../BootMaintenanceManagerUiLib/BootOption.c | 25 +++++++++------- .../Library/FileExplorerLib/FileExplorer.c | 5 +++- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 33 ++++++++++++---------- .../Universal/DisplayEngineDxe/ProcessOptions.c | 11 ++++---- MdeModulePkg/Universal/HiiDatabaseDxe/Font.c | 14 +++++---- .../Universal/Variable/RuntimeDxe/Variable.c | 2 +- 9 files changed, 80 insertions(+), 56 deletions(-) (limited to 'MdeModulePkg') diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c index 843815d..14bed54 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 4874108..496ffbd 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 5903ce7..41af50b 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 cdaa2db..e22aaf3 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 ef94926..804a03d 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 766dde3..72de8d3 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 dca3c1d..0d4cfa4 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 399f90f..8a0b12f 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 14c1768..3eb7d93 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; } -- cgit v1.1