MdeModulePkg: CodeQL Fixes.

Includes changes across the repo for the following CodeQL rules: - cpp/comparison-with-wider-type - cpp/overflow-buffer - cpp/redundant-null-check-param - cpp/uselesstest Co-authored-by: Taylor Beebe <tabeebe@microsoft.com> Co-authored-by: pohanch <125842322+pohanch@users.noreply.github.com> Co-authored-by: kenlautner <85201046+kenlautner@users.noreply.github.com> Co-authored-by: Oliver Smith-Denny <osde@linux.microsoft.com> Co-authored-by: Sean Brogan <sean.brogan@microsoft.com> Co-authored-by: Aaron <105021049+apop5@users.noreply.github.com>
microsoft · Sep 10, 2024 · 88b6c1e · 88b6c1e
1 parent 91a2538
commit 88b6c1e
Show file tree

Hide file tree

Showing 132 changed files with 4,224 additions and 1,864 deletions.
diff --git a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.c b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.c
@@ -1096,69 +1096,74 @@ BootManagerMenuEntry (
   // Initialize Boot menu data
   //
   Status = InitializeBootMenuData (BootOption, BootOptionCount, &BootMenuData);
-  //
-  // According to boot menu data to draw boot popup menu
-  //
-  DrawBootPopupMenu (&BootMenuData);
-
-  //
-  // check user input to determine want to re-draw or boot from user selected item
-  //
-  ExitApplication = FALSE;
-  while (!ExitApplication) {
-    gBS->WaitForEvent (1, &gST->ConIn->WaitForKey, &Index);
-    Status = gST->ConIn->ReadKeyStroke (gST->ConIn, &Key);
-    if (!EFI_ERROR (Status)) {
-      switch (Key.UnicodeChar) {
-        case CHAR_NULL:
-          switch (Key.ScanCode) {
-            case SCAN_UP:
-              SelectItem = BootMenuData.SelectItem == 0 ? BootMenuData.ItemCount - 1 : BootMenuData.SelectItem - 1;
-              BootMenuSelectItem (SelectItem, &BootMenuData);
-              break;
-
-            case SCAN_DOWN:
-              SelectItem = BootMenuData.SelectItem == BootMenuData.ItemCount - 1 ? 0 : BootMenuData.SelectItem + 1;
-              BootMenuSelectItem (SelectItem, &BootMenuData);
-              break;
-
-            case SCAN_ESC:
-              gST->ConOut->ClearScreen (gST->ConOut);
-              ExitApplication = TRUE;
-              //
-              // Set boot resolution for normal boot
-              //
-              BdsSetConsoleMode (FALSE);
-              break;
+  // MU_CHANGE Start - CodeQL Change - Verify InitializeBootMenuData returned valid data
+  if (!EFI_ERROR (Status)) {
+    //
+    // According to boot menu data to draw boot popup menu
+    //
+    DrawBootPopupMenu (&BootMenuData);
 
-            default:
-              break;
-          }
+    //
+    // check user input to determine want to re-draw or boot from user selected item
+    //
+    ExitApplication = FALSE;
+    while (!ExitApplication) {
+      gBS->WaitForEvent (1, &gST->ConIn->WaitForKey, &Index);
+      Status = gST->ConIn->ReadKeyStroke (gST->ConIn, &Key);
+      if (!EFI_ERROR (Status)) {
+        switch (Key.UnicodeChar) {
+          case CHAR_NULL:
+            switch (Key.ScanCode) {
+              case SCAN_UP:
+                SelectItem = BootMenuData.SelectItem == 0 ? BootMenuData.ItemCount - 1 : BootMenuData.SelectItem - 1;
+                BootMenuSelectItem (SelectItem, &BootMenuData);
+                break;
+
+              case SCAN_DOWN:
+                SelectItem = BootMenuData.SelectItem == BootMenuData.ItemCount - 1 ? 0 : BootMenuData.SelectItem + 1;
+                BootMenuSelectItem (SelectItem, &BootMenuData);
+                break;
+
+              case SCAN_ESC:
+                gST->ConOut->ClearScreen (gST->ConOut);
+                ExitApplication = TRUE;
+                //
+                // Set boot resolution for normal boot
+                //
+                BdsSetConsoleMode (FALSE);
+                break;
+
+              default:
+                break;
+            }
 
-          break;
+            break;
 
-        case CHAR_CARRIAGE_RETURN:
-          gST->ConOut->ClearScreen (gST->ConOut);
-          //
-          // Set boot resolution for normal boot
-          //
-          BdsSetConsoleMode (FALSE);
-          BootFromSelectOption (BootOption, BootOptionCount, BootMenuData.SelectItem);
-          //
-          // Back to boot manager menu again, set back to setup resolution
-          //
-          BdsSetConsoleMode (TRUE);
-          DrawBootPopupMenu (&BootMenuData);
-          break;
+          case CHAR_CARRIAGE_RETURN:
+            gST->ConOut->ClearScreen (gST->ConOut);
+            //
+            // Set boot resolution for normal boot
+            //
+            BdsSetConsoleMode (FALSE);
+            BootFromSelectOption (BootOption, BootOptionCount, BootMenuData.SelectItem);
+            //
+            // Back to boot manager menu again, set back to setup resolution
+            //
+            BdsSetConsoleMode (TRUE);
+            DrawBootPopupMenu (&BootMenuData);
+            break;
 
-        default:
-          break;
+          default:
+            break;
+        }
       }
     }
+
+    EfiBootManagerFreeLoadOptions (BootOption, BootOptionCount);
+    FreePool (BootMenuData.PtrTokens);
   }
 
-  EfiBootManagerFreeLoadOptions (BootOption, BootOptionCount);
-  FreePool (BootMenuData.PtrTokens);
+  // MU_CHANGE End - CodeQL Change - Verify InitializeBootMenuData returned valid data
 
   HiiRemovePackages (gStringPackHandle);
 

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
@@ -993,7 +993,8 @@ DumpProvisionedCapsule (
       //
       // Display description and device path
       //
-      GetEfiSysPartitionFromBootOptionFilePath (BootNextOptionEntry.FilePath, &DevicePath, &Fs);
+      // MU_CHANGE - CodeQL Change
+      Status = GetEfiSysPartitionFromBootOptionFilePath (BootNextOptionEntry.FilePath, &DevicePath, &Fs);
       if (!EFI_ERROR (Status)) {
         Print (L"Capsules are provisioned on BootOption: %s\n", BootNextOptionEntry.Description);
         Print (L"    %s %s\n", ShellProtocol->GetMapFromDevicePath (&DevicePath), ConvertDevicePathToText (DevicePath, TRUE, TRUE));

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c b/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
@@ -518,7 +518,12 @@ GetUpdateFileSystem (
     // If map is assigned, try to get ESP from mapped Fs.
     //
     DevicePath = DuplicateDevicePath (MappedDevicePath);
-    Status     = GetEfiSysPartitionFromDevPath (DevicePath, &FullPath, Fs);
+    // MU_CHANGE - CodeQl Change
+    if (DevicePath == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    Status = GetEfiSysPartitionFromDevPath (DevicePath, &FullPath, Fs);
     if (EFI_ERROR (Status)) {
       Print (L"Error: Cannot get EFI system partition from '%s' - %r\n", Map, Status);
       return EFI_NOT_FOUND;

diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
@@ -614,7 +614,13 @@ DumpSmiHandler (
 
         Print (L">\n");
         ImageStruct = GetImageFromRef ((UINTN)SmiHandlerStruct->ImageRef);
-        NameString  = GetDriverNameString (ImageStruct);
+        // MU_CHANGE - CodeQl Changes - If ImageStruct returned NULL, initialize NameString to an empty string
+        if (ImageStruct != NULL) {
+          NameString = GetDriverNameString (ImageStruct);
+        } else {
+          NameString = "\0";
+        }
+
         Print (L"      <Module RefId=\"0x%x\" Name=\"%a\">\n", SmiHandlerStruct->ImageRef, NameString);
         if ((ImageStruct != NULL) && (ImageStruct->PdbStringOffset != 0)) {
           Print (L"      <Pdb>%a</Pdb>\n", (UINT8 *)ImageStruct + ImageStruct->PdbStringOffset);

diff --git a/MdeModulePkg/Application/UiApp/FrontPage.c b/MdeModulePkg/Application/UiApp/FrontPage.c
@@ -205,40 +205,44 @@ UpdateFrontPageForm (
   //
   StartOpCodeHandle = HiiAllocateOpCodeHandle ();
   ASSERT (StartOpCodeHandle != NULL);
+  // MU_CHANGE Start - CodeQl Change - Handle StartOpCodeHandle and EndOpCodeHandle allocation failures
+  if (StartOpCodeHandle != NULL) {
+    EndOpCodeHandle = HiiAllocateOpCodeHandle ();
+    ASSERT (EndOpCodeHandle != NULL);
+    if (EndOpCodeHandle != NULL) {
+      //
+      // Create Hii Extend Label OpCode as the start opcode
+      //
+      StartGuidLabel               = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode (StartOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL));
+      StartGuidLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
+      StartGuidLabel->Number       = LABEL_FRONTPAGE_INFORMATION;
+      //
+      // Create Hii Extend Label OpCode as the end opcode
+      //
+      EndGuidLabel               = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode (EndOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL));
+      EndGuidLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
+      EndGuidLabel->Number       = LABEL_END;
 
-  EndOpCodeHandle = HiiAllocateOpCodeHandle ();
-  ASSERT (EndOpCodeHandle != NULL);
-  //
-  // Create Hii Extend Label OpCode as the start opcode
-  //
-  StartGuidLabel               = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode (StartOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL));
-  StartGuidLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
-  StartGuidLabel->Number       = LABEL_FRONTPAGE_INFORMATION;
-  //
-  // Create Hii Extend Label OpCode as the end opcode
-  //
-  EndGuidLabel               = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode (EndOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL));
-  EndGuidLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
-  EndGuidLabel->Number       = LABEL_END;
-
-  //
-  // Updata Front Page form
-  //
-  UiCustomizeFrontPage (
-    gFrontPagePrivate.HiiHandle,
-    StartOpCodeHandle
-    );
-
-  HiiUpdateForm (
-    gFrontPagePrivate.HiiHandle,
-    &mFrontPageGuid,
-    FRONT_PAGE_FORM_ID,
-    StartOpCodeHandle,
-    EndOpCodeHandle
-    );
+      //
+      // Updata Front Page form
+      //
+      UiCustomizeFrontPage (
+        gFrontPagePrivate.HiiHandle,
+        StartOpCodeHandle
+        );
+
+      HiiUpdateForm (
+        gFrontPagePrivate.HiiHandle,
+        &mFrontPageGuid,
+        FRONT_PAGE_FORM_ID,
+        StartOpCodeHandle,
+        EndOpCodeHandle
+        );
+      HiiFreeOpCodeHandle (EndOpCodeHandle);
+    }
 
-  HiiFreeOpCodeHandle (StartOpCodeHandle);
-  HiiFreeOpCodeHandle (EndOpCodeHandle);
+    HiiFreeOpCodeHandle (StartOpCodeHandle);
+  } // // MU_CHANGE End - CodeQl Change - Handle StartOpCodeHandle and EndOpCodeHandle allocation failures
 }
 
 /**
@@ -976,7 +980,10 @@ InitializeUserInterface (
   UiSetConsoleMode (FALSE);
 
   UninitializeStringSupport ();
-  HiiRemovePackages (HiiHandle);
+  // MU_CHANGE Start - CodeQl Change - Deal with HiiHandle being NULL
+  if (HiiHandle != NULL) {
+    HiiRemovePackages (HiiHandle);
+  }
 
   return EFI_SUCCESS;
 }

diff --git a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
@@ -191,6 +191,12 @@ UiSupportLibCallbackHandler (
 
   if (Action == EFI_BROWSER_ACTION_RETRIEVE) {
     if (QuestionId == FRONT_PAGE_KEY_LANGUAGE) {
+      // MU_CHANGE Start - CodeQl Change
+      if (Value == NULL) {
+        *Status = EFI_INVALID_PARAMETER;
+        return FALSE;
+      }
+
       Value->u8 = gCurrentLanguageIndex;
       *Status   = EFI_SUCCESS;
     } else {
@@ -517,6 +523,8 @@ RequiredDriver (
   UINTN             TempSize;
   BOOLEAN           RetVal;
 
+  Buffer = NULL; // MU_CHANGE - CodeQl Change
+
   Status = HiiGetFormSetFromHiiHandle (HiiHandle, &Buffer, &BufferSize);
   if (EFI_ERROR (Status)) {
     return FALSE;

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
@@ -943,7 +943,7 @@ AtaPioDataInOut (
   IN     ATA_NONBLOCK_TASK      *Task
   )
 {
-  UINTN       WordCount;
+  UINT64      WordCount; // MU_CHANGE - CodeQL Change - comparison mismatch
   UINTN       Increment;
   UINT16      *Buffer16;
   EFI_STATUS  Status;

diff --git a/MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c b/MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c
@@ -563,7 +563,8 @@ AtapiEnumerateDevices (
   //
   // Using Command and Control Regs Base Address to fill other registers.
   //
-  for (Index1 = 0; Index1 < IdeEnabledNumber; Index1++) {
+  for (Index1 = 0; (UINT32)Index1 < IdeEnabledNumber; Index1++) {
+    // MU_CHANGE - CodeQl Change
     CommandBlockBaseAddr                             = IdeRegsBaseAddr[Index1].CommandBlockBaseAddr;
     AtapiBlkIoDev->IdeIoPortReg[Index1].Data         = CommandBlockBaseAddr;
     AtapiBlkIoDev->IdeIoPortReg[Index1].Reg1.Feature = (UINT16)(CommandBlockBaseAddr + 0x1);

diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -289,6 +289,8 @@ PciIoMemRead (
   EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *Desc;
   EFI_STATUS                         Status;
 
+  Desc = NULL; // MU_CHANGE - CodeQl Change
+
   if (Buffer == NULL) {
     return EFI_INVALID_PARAMETER;
   }
@@ -377,6 +379,8 @@ PciIoMemWrite (
   EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *Desc;
   EFI_STATUS                         Status;
 
+  Desc = NULL; // MS_CHANGE for vs2017
+
   if (Buffer == NULL) {
     return EFI_INVALID_PARAMETER;
   }
@@ -1111,6 +1115,8 @@ NonCoherentPciIoAllocateBuffer (
   NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION  *Alloc;
   VOID                                         *AllocAddress;
 
+  AllocAddress = NULL; // MS_CHANGE for vs2017
+
   if (HostAddress == NULL) {
     return EFI_INVALID_PARAMETER;
   }
@@ -1243,6 +1249,8 @@ NonCoherentPciIoMap (
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR       GcdDescriptor;
   BOOLEAN                               Bounce;
 
+  AllocAddress = NULL; // MS_CHANGE for vs2017
+
   if ((HostAddress   == NULL) ||
       (NumberOfBytes == NULL) ||
       (DeviceAddress == NULL) ||
@@ -1610,6 +1618,8 @@ PciIoGetBarAttributes (
   EFI_ACPI_END_TAG_DESCRIPTOR        *End;
   EFI_STATUS                         Status;
 
+  BarDesc = NULL; // MS_CHANGE for vs2017
+
   if ((Supports == NULL) && (Resources == NULL)) {
     return EFI_INVALID_PARAMETER;
   }

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -219,10 +219,10 @@ NvmeCreatePrpList (
   OUT VOID                     **Mapping
   )
 {
-  UINTN                 PrpEntryNo;
+  UINT64                PrpEntryNo; // MU_CHANGE - CodeQl Change - comparison mismatch
   UINT64                PrpListBase;
-  UINTN                 PrpListIndex;
-  UINTN                 PrpEntryIndex;
+  UINT64                PrpListIndex;  // MU_CHANGE - CodeQl Change - comparison mismatch
+  UINT64                PrpEntryIndex; // MU_CHANGE - CodeQl Change - comparison mismatch
   UINT64                Remainder;
   EFI_PHYSICAL_ADDRESS  PrpListPhyAddr;
   UINTN                 Bytes;

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c
@@ -564,14 +564,18 @@ NvmeControllerInit (
   //
   // Dump the NVME controller implementation version
   //
-  NVME_GET_VER (Private, &Ver);
-  DEBUG ((DEBUG_INFO, "NVME controller implementation version: %d.%d\n", Ver.Mjr, Ver.Mnr));
+  // MU_CHANGE - CodeQl Change - Check return status of NVME_GET_VER macro
+  Status = NVME_GET_VER (Private, &Ver);
+  if (!EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "NVME controller implementation version: %d.%d\n", Ver.Mjr, Ver.Mnr));
+  }
 
   //
   // Read the controller Capabilities register and verify that the NVM command set is supported
   //
-  NVME_GET_CAP (Private, &Private->Cap);
-  if ((Private->Cap.Css & BIT0) == 0) {
+  // MU_CHANGE - CodeQl Change - Check return status of NVME_GET_CAP macro
+  Status = NVME_GET_CAP (Private, &Private->Cap);
+  if ( !EFI_ERROR (Status) && ((Private->Cap.Css & BIT0) == 0)) {
     DEBUG ((DEBUG_ERROR, "%a: The NVME controller doesn't support NVMe command set.\n", __func__));
     return EFI_UNSUPPORTED;
   }

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c
@@ -27,12 +27,12 @@ NvmeCreatePrpList (
   IN     UINTN                             Pages
   )
 {
-  UINTN                 PrpEntryNo;
+  UINT64                PrpEntryNo; // MU_CHANGE - CodeQl Change - comparison mismatch
   UINTN                 PrpListNo;
   UINT64                PrpListBase;
   VOID                  *PrpListHost;
   UINTN                 PrpListIndex;
-  UINTN                 PrpEntryIndex;
+  UINT64                PrpEntryIndex; // MU_CHANGE - CodeQl Change - comparison mismatch
   UINT64                Remainder;
   EFI_PHYSICAL_ADDRESS  PrpListPhyAddr;
   UINTN                 Bytes;