From 0bc1db4adfbdf0160a9c60eec18a23f3e282f4ab Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Wed, 22 Oct 2025 11:33:02 -0700 Subject: [PATCH] MdeModulePkg: Fix missing NULL tests https://github.com/github/codeql/blob/codeql-cli-2.7.3/cpp/ql/src/Critical/MissingNullTest.qhelp For items which allocate memory, or get a pointer from another structure, it is important to validate that the pointers are not null before they are dereferenced. Signed-off-by: Aaron Pop --- .../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 15 +- .../NonDiscoverablePciDeviceIo.c | 2 +- .../Bus/Pci/PciBusDxe/PciDeviceSupport.c | 6 +- .../Bus/Pci/PciBusDxe/PciEnumerator.c | 59 ++++-- MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 45 ++++- .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 4 +- .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 183 ++++++++++-------- .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 5 +- .../Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 21 +- MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 28 ++- MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 48 ++++- MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 44 ++++- 12 files changed, 328 insertions(+), 132 deletions(-) diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c index 57aefa04c2..bcd5b374c2 100644 --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c @@ -929,12 +929,19 @@ EXIT: if (EFI_ERROR (Status)) { OldTpl = gBS->RaiseTPL (TPL_NOTIFY); Token->TransactionStatus = Status; - *EventCount = (*EventCount) - (TempCount - Index); - *IsError = TRUE; + if (EventCount != NULL) { + *EventCount = (*EventCount) - (TempCount - Index); + } - if (*EventCount == 0) { + if (IsError != NULL) { + *IsError = TRUE; + } + + if ((EventCount != NULL) && (*EventCount == 0)) { FreePool (EventCount); - FreePool (IsError); + if (IsError != NULL) { + FreePool (IsError); + } } if (SubTask != NULL) { diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c index 4daf51761b..8cf80e5c9d 100644 --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c @@ -1008,7 +1008,7 @@ NonCoherentPciIoFreeBuffer ( } } - if (!Found) { + if (!Found || (Alloc == NULL)) { ASSERT_EFI_ERROR (EFI_NOT_FOUND); return EFI_NOT_FOUND; } diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c index 409c01d107..3d3e064f37 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c @@ -781,7 +781,11 @@ StartPciDevices ( LIST_ENTRY *CurrentLink; RootBridge = GetRootBridgeByHandle (Controller); - ASSERT (RootBridge != NULL); + if (RootBridge == NULL ) { + ASSERT (RootBridge != NULL); + return EFI_NOT_READY; + } + ThisHostBridge = RootBridge->PciRootBridgeIo->ParentHandle; CurrentLink = mPciDevicePool.ForwardLink; diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c index 3f8c6e6da7..f49ff4f798 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c @@ -881,7 +881,9 @@ GetMaxResourceConsumerDevice ( && (Temp->ResourceUsage != PciResUsagePadding)) { PPBResNode = GetMaxResourceConsumerDevice (Temp); - PciResNode = GetLargerConsumerDevice (PciResNode, PPBResNode); + if (PPBResNode != NULL) { + PciResNode = GetLargerConsumerDevice (PciResNode, PPBResNode); + } } else { PciResNode = GetLargerConsumerDevice (PciResNode, Temp); } @@ -1445,6 +1447,8 @@ PciBridgeResourceAllocator ( UINT64 PMem64Base; EFI_STATUS Status; + Status = EFI_OUT_OF_RESOURCES; + IoBridge = CreateResourceNode ( Bridge, 0, @@ -1453,6 +1457,9 @@ PciBridgeResourceAllocator ( PciBarTypeIo16, PciResUsageTypical ); + if (IoBridge == NULL) { + goto Exit; + } Mem32Bridge = CreateResourceNode ( Bridge, @@ -1462,6 +1469,9 @@ PciBridgeResourceAllocator ( PciBarTypeMem32, PciResUsageTypical ); + if (Mem32Bridge == NULL) { + goto Exit1; + } PMem32Bridge = CreateResourceNode ( Bridge, @@ -1471,6 +1481,9 @@ PciBridgeResourceAllocator ( PciBarTypePMem32, PciResUsageTypical ); + if (PMem32Bridge == NULL) { + goto Exit2; + } Mem64Bridge = CreateResourceNode ( Bridge, @@ -1480,6 +1493,9 @@ PciBridgeResourceAllocator ( PciBarTypeMem64, PciResUsageTypical ); + if (Mem64Bridge == NULL) { + goto Exit3; + } PMem64Bridge = CreateResourceNode ( Bridge, @@ -1489,6 +1505,9 @@ PciBridgeResourceAllocator ( PciBarTypePMem64, PciResUsageTypical ); + if (PMem64Bridge == NULL) { + goto Exit4; + } // // Create resourcemap by going through all the devices subject to this root bridge @@ -1512,7 +1531,7 @@ PciBridgeResourceAllocator ( ); if (EFI_ERROR (Status)) { - return Status; + goto Exit5; } // @@ -1555,19 +1574,29 @@ PciBridgeResourceAllocator ( PMem64Bridge ); - DestroyResourceTree (IoBridge); - DestroyResourceTree (Mem32Bridge); - DestroyResourceTree (PMem32Bridge); +Exit5: DestroyResourceTree (PMem64Bridge); - DestroyResourceTree (Mem64Bridge); - - gBS->FreePool (IoBridge); - gBS->FreePool (Mem32Bridge); - gBS->FreePool (PMem32Bridge); gBS->FreePool (PMem64Bridge); + +Exit4: + DestroyResourceTree (Mem64Bridge); gBS->FreePool (Mem64Bridge); - return EFI_SUCCESS; +Exit3: + DestroyResourceTree (PMem32Bridge); + gBS->FreePool (PMem32Bridge); + +Exit2: + DestroyResourceTree (Mem32Bridge); + gBS->FreePool (Mem32Bridge); + +Exit1: + DestroyResourceTree (IoBridge); + gBS->FreePool (IoBridge); + +Exit: + + return Status; } /** @@ -2015,12 +2044,10 @@ PciHotPlugRequestNotify ( return EFI_INVALID_PARAMETER; } - if (Operation == EfiPciHotPlugRequestAdd) { - if (ChildHandleBuffer == NULL) { + if (ChildHandleBuffer == NULL) { + if (Operation == EfiPciHotPlugRequestAdd) { return EFI_INVALID_PARAMETER; - } - } else if ((Operation == EfiPciHotplugRequestRemove) && (*NumberOfChildren != 0)) { - if (ChildHandleBuffer == NULL) { + } else if ((Operation == EfiPciHotplugRequestRemove) && (*NumberOfChildren != 0)) { return EFI_INVALID_PARAMETER; } } diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c index e75b18eaa1..5b4b77ed07 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c @@ -381,6 +381,10 @@ DumpResourceMap ( } ChildResources = AllocatePool (sizeof (PCI_RESOURCE_NODE *) * ChildResourceCount); + if (ChildResources == NULL) { + return; + } + ASSERT (ChildResources != NULL); ChildResourceCount = 0; for (Index = 0; Index < ResourceCount; Index++) { @@ -523,7 +527,6 @@ PciHostBridgeResourceAllocator ( // Get Root Bridge Device by handle // RootBridgeDev = GetRootBridgeByHandle (RootBridgeHandle); - if (RootBridgeDev == NULL) { return EFI_NOT_FOUND; } @@ -545,6 +548,9 @@ PciHostBridgeResourceAllocator ( PciBarTypeIo16, PciResUsageTypical ); + if (IoBridge == NULL) { + return EFI_OUT_OF_RESOURCES; + } Mem32Bridge = CreateResourceNode ( RootBridgeDev, @@ -554,6 +560,10 @@ PciHostBridgeResourceAllocator ( PciBarTypeMem32, PciResUsageTypical ); + if (Mem32Bridge == NULL) { + FreePool (IoBridge); + return EFI_OUT_OF_RESOURCES; + } PMem32Bridge = CreateResourceNode ( RootBridgeDev, @@ -563,6 +573,11 @@ PciHostBridgeResourceAllocator ( PciBarTypePMem32, PciResUsageTypical ); + if (PMem32Bridge == NULL) { + FreePool (IoBridge); + FreePool (Mem32Bridge); + return EFI_OUT_OF_RESOURCES; + } Mem64Bridge = CreateResourceNode ( RootBridgeDev, @@ -572,6 +587,12 @@ PciHostBridgeResourceAllocator ( PciBarTypeMem64, PciResUsageTypical ); + if (Mem64Bridge == NULL) { + FreePool (IoBridge); + FreePool (Mem32Bridge); + FreePool (PMem32Bridge); + return EFI_OUT_OF_RESOURCES; + } PMem64Bridge = CreateResourceNode ( RootBridgeDev, @@ -581,6 +602,13 @@ PciHostBridgeResourceAllocator ( PciBarTypePMem64, PciResUsageTypical ); + if (PMem64Bridge == NULL) { + FreePool (IoBridge); + FreePool (Mem32Bridge); + FreePool (PMem32Bridge); + FreePool (Mem64Bridge); + return EFI_OUT_OF_RESOURCES; + } // // Get the max ROM size that the root bridge can process @@ -667,10 +695,13 @@ PciHostBridgeResourceAllocator ( } } - // - // End while, at least one Root Bridge should be found. - // - ASSERT (RootBridgeDev != NULL); + if (RootBridgeDev == NULL) { + // + // End while, at least one Root Bridge should be found. + // + ASSERT (RootBridgeDev != NULL); + return EFI_NOT_FOUND; + } // // Notify platform to start to program the resource @@ -764,6 +795,10 @@ PciHostBridgeResourceAllocator ( } } + if (RootBridgeDev == NULL) { + return EFI_NOT_FOUND; + } + // // End while // diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c index c290884110..1e5f5212a3 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c @@ -727,7 +727,9 @@ ProcessOpRomImage ( EfiOpRomImageNode.EndingOffset = (UINTN)RomBarOffset + ImageSize - 1 - (UINTN)RomBar; PciOptionRomImageDevicePath = AppendDevicePathNode (PciDevice->DevicePath, &EfiOpRomImageNode.Header); - ASSERT (PciOptionRomImageDevicePath != NULL); + if (PciOptionRomImageDevicePath == NULL) { + return EFI_NOT_FOUND; + } // // load image and start image diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c index 8ffd05f327..8f3cff3eaa 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c @@ -430,12 +430,15 @@ GetResourceFromDevice ( { UINT8 Index; PCI_RESOURCE_NODE *Node; + PCI_RESOURCE_NODE *DestNode; BOOLEAN ResourceRequested; Node = NULL; ResourceRequested = FALSE; for (Index = 0; Index < PCI_MAX_BAR; Index++) { + DestNode = NULL; + Node = NULL; switch ((PciDev->PciBar)[Index].BarType) { case PciBarTypeMem32: case PciBarTypeOpRom: @@ -448,13 +451,8 @@ GetResourceFromDevice ( (PciDev->PciBar)[Index].BarType, PciResUsageTypical ); + DestNode = Mem32Node; - InsertResourceNode ( - Mem32Node, - Node - ); - - ResourceRequested = TRUE; break; case PciBarTypeMem64: @@ -467,13 +465,8 @@ GetResourceFromDevice ( PciBarTypeMem64, PciResUsageTypical ); + DestNode = Mem64Node; - InsertResourceNode ( - Mem64Node, - Node - ); - - ResourceRequested = TRUE; break; case PciBarTypePMem64: @@ -486,13 +479,8 @@ GetResourceFromDevice ( PciBarTypePMem64, PciResUsageTypical ); + DestNode = PMem64Node; - InsertResourceNode ( - PMem64Node, - Node - ); - - ResourceRequested = TRUE; break; case PciBarTypePMem32: @@ -505,12 +493,7 @@ GetResourceFromDevice ( PciBarTypePMem32, PciResUsageTypical ); - - InsertResourceNode ( - PMem32Node, - Node - ); - ResourceRequested = TRUE; + DestNode = PMem32Node; break; case PciBarTypeIo16: @@ -524,12 +507,8 @@ GetResourceFromDevice ( PciBarTypeIo16, PciResUsageTypical ); + DestNode = IoNode; - InsertResourceNode ( - IoNode, - Node - ); - ResourceRequested = TRUE; break; case PciBarTypeUnknown: @@ -538,12 +517,20 @@ GetResourceFromDevice ( default: break; } + + if ((DestNode != NULL) && (Node != NULL)) { + InsertResourceNode (DestNode, Node); + ResourceRequested = TRUE; + } } // // Add VF resource // for (Index = 0; Index < PCI_MAX_BAR; Index++) { + DestNode = NULL; + Node = NULL; + switch ((PciDev->VfPciBar)[Index].BarType) { case PciBarTypeMem32: @@ -555,11 +542,7 @@ GetResourceFromDevice ( PciBarTypeMem32, PciResUsageTypical ); - - InsertResourceNode ( - Mem32Node, - Node - ); + DestNode = Mem32Node; break; @@ -573,11 +556,7 @@ GetResourceFromDevice ( PciBarTypeMem64, PciResUsageTypical ); - - InsertResourceNode ( - Mem64Node, - Node - ); + DestNode = Mem64Node; break; @@ -591,12 +570,7 @@ GetResourceFromDevice ( PciBarTypePMem64, PciResUsageTypical ); - - InsertResourceNode ( - PMem64Node, - Node - ); - + DestNode = PMem64Node; break; case PciBarTypePMem32: @@ -609,11 +583,8 @@ GetResourceFromDevice ( PciBarTypePMem32, PciResUsageTypical ); + DestNode = PMem32Node; - InsertResourceNode ( - PMem32Node, - Node - ); break; case PciBarTypeIo16: @@ -626,6 +597,10 @@ GetResourceFromDevice ( default: break; } + + if ((DestNode != NULL) && (Node != NULL)) { + InsertResourceNode (DestNode, Node); + } } // If there is no resource requested from this device, @@ -783,6 +758,9 @@ CreateResourceMap ( PciBarTypeIo16, PciResUsageTypical ); + if (IoBridge == NULL) { + return; + } Mem32Bridge = CreateResourceNode ( Temp, @@ -792,6 +770,10 @@ CreateResourceMap ( PciBarTypeMem32, PciResUsageTypical ); + if (Mem32Bridge == NULL) { + FreePool (IoBridge); + return; + } PMem32Bridge = CreateResourceNode ( Temp, @@ -801,6 +783,11 @@ CreateResourceMap ( PciBarTypePMem32, PciResUsageTypical ); + if (PMem32Bridge == NULL) { + FreePool (Mem32Bridge); + FreePool (IoBridge); + return; + } Mem64Bridge = CreateResourceNode ( Temp, @@ -810,6 +797,12 @@ CreateResourceMap ( PciBarTypeMem64, PciResUsageTypical ); + if (Mem64Bridge == NULL) { + FreePool (PMem32Bridge); + FreePool (Mem32Bridge); + FreePool (IoBridge); + return; + } PMem64Bridge = CreateResourceNode ( Temp, @@ -819,6 +812,13 @@ CreateResourceMap ( PciBarTypePMem64, PciResUsageTypical ); + if (PMem64Bridge == NULL) { + FreePool (Mem64Bridge); + FreePool (PMem32Bridge); + FreePool (Mem32Bridge); + FreePool (IoBridge); + return; + } // // Recursively create resource map on this bridge @@ -1813,12 +1813,11 @@ ResourcePaddingForCardBusBridge ( PciBarTypeMem32, PciResUsagePadding ); + if (Node == NULL) { + return; + } - InsertResourceNode ( - Mem32Node, - Node - ); - + InsertResourceNode (Mem32Node, Node); // // Memory Base/Limit Register 1 // Bar 2 decodes memory range1 @@ -1831,11 +1830,11 @@ ResourcePaddingForCardBusBridge ( PciBarTypePMem32, PciResUsagePadding ); + if (Node == NULL) { + return; + } - InsertResourceNode ( - PMem32Node, - Node - ); + InsertResourceNode (PMem32Node, Node); // // Io Base/Limit @@ -1850,10 +1849,11 @@ ResourcePaddingForCardBusBridge ( PciResUsagePadding ); - InsertResourceNode ( - IoNode, - Node - ); + if (Node == NULL) { + return; + } + + InsertResourceNode (IoNode, Node); // // Io Base/Limit @@ -1867,11 +1867,11 @@ ResourcePaddingForCardBusBridge ( PciBarTypeIo16, PciResUsagePadding ); + if (Node == NULL) { + return; + } - InsertResourceNode ( - IoNode, - Node - ); + InsertResourceNode (IoNode, Node); } /** @@ -2142,10 +2142,13 @@ ApplyResourcePadding ( PciBarTypeIo16, PciResUsagePadding ); - InsertResourceNode ( - IoNode, - Node - ); + + if (Node != NULL) { + InsertResourceNode ( + IoNode, + Node + ); + } } Ptr++; @@ -2167,10 +2170,12 @@ ApplyResourcePadding ( PciBarTypePMem32, PciResUsagePadding ); - InsertResourceNode ( - PMem32Node, - Node - ); + if (Node != NULL) { + InsertResourceNode ( + PMem32Node, + Node + ); + } } Ptr++; @@ -2190,10 +2195,12 @@ ApplyResourcePadding ( PciBarTypeMem32, PciResUsagePadding ); - InsertResourceNode ( - Mem32Node, - Node - ); + if (Node != NULL) { + InsertResourceNode ( + Mem32Node, + Node + ); + } } Ptr++; @@ -2215,10 +2222,12 @@ ApplyResourcePadding ( PciBarTypePMem64, PciResUsagePadding ); - InsertResourceNode ( - PMem64Node, - Node - ); + if (Node != NULL) { + InsertResourceNode ( + PMem64Node, + Node + ); + } } Ptr++; @@ -2238,10 +2247,12 @@ ApplyResourcePadding ( PciBarTypeMem64, PciResUsagePadding ); - InsertResourceNode ( - Mem64Node, - Node - ); + if (Node != NULL) { + InsertResourceNode ( + Mem64Node, + Node + ); + } } Ptr++; diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c index 120aef31c0..e9e827887a 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c @@ -667,7 +667,10 @@ ResourceConflict ( RootBridgeCount * (TypeMax * sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR)) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR) ); - ASSERT (Resources != NULL); + if (Resources == NULL) { + ASSERT (Resources != NULL); + return; + } for (Link = GetFirstNode (&HostBridge->RootBridges), Descriptor = Resources ; !IsNull (&HostBridge->RootBridges, Link) diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c index 419f47a426..2dfaa22b57 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c @@ -196,7 +196,11 @@ CreateRootBridge ( } RootBridge = AllocateZeroPool (sizeof (PCI_ROOT_BRIDGE_INSTANCE)); - ASSERT (RootBridge != NULL); + if (RootBridge == NULL) { + DEBUG ((DEBUG_ERROR, "Failed to allocate RootBridge\n")); + ASSERT (RootBridge != NULL); + return NULL; + } RootBridge->Signature = PCI_ROOT_BRIDGE_SIGNATURE; RootBridge->Supports = Bridge->Supports; @@ -209,7 +213,12 @@ CreateRootBridge ( RootBridge->ConfigBuffer = AllocatePool ( TypeMax * sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR) ); - ASSERT (RootBridge->ConfigBuffer != NULL); + if (RootBridge->ConfigBuffer == NULL) { + ASSERT (RootBridge->ConfigBuffer != NULL); + FreePool (RootBridge); + return NULL; + } + InitializeListHead (&RootBridge->Maps); CopyMem (&RootBridge->Bus, &Bridge->Bus, sizeof (PCI_ROOT_BRIDGE_APERTURE)); @@ -245,6 +254,14 @@ CreateRootBridge ( break; } + if (Aperture == NULL) { + DEBUG ((DEBUG_ERROR, "%a - Failed to Get Resource!!!\n", __func__)); + DEBUG ((DEBUG_ERROR, "No EFI_PCI_ROOT_BRIDGE_INSTANCE created\n")); + FreePool (RootBridge->ConfigBuffer); + FreePool (RootBridge); + return NULL; + } + RootBridge->ResAllocNode[Index].Type = Index; if (Bridge->ResourceAssigned && (Aperture->Limit >= Aperture->Base)) { // diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c index 8b1ce70118..1f522145d1 100644 --- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c +++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c @@ -758,7 +758,10 @@ GetChildSerialDevices ( } SerialDevices = AllocatePool (EntryCount * sizeof (SERIAL_DEV *)); - ASSERT (SerialDevices != NULL); + if (SerialDevices == NULL) { + ASSERT (SerialDevices != NULL); + return NULL; + } *Count = 0; OpenByDriver = FALSE; @@ -905,7 +908,11 @@ SerialControllerDriverStart ( // Uart = (UART_DEVICE_PATH *)SkipControllerDevicePathNode (RemainingDevicePath, &ContainsControllerNode, &ControllerNumber); for (Index = 0; Index < SerialDeviceCount; Index++) { - ASSERT ((SerialDevices != NULL) && (SerialDevices[Index] != NULL)); + if ((SerialDevices == NULL) || (SerialDevices[Index] == NULL)) { + ASSERT ((SerialDevices != NULL) && (SerialDevices[Index] != NULL)); + continue; + } + if ((!SerialDevices[Index]->ContainsControllerNode && !ContainsControllerNode) || (SerialDevices[Index]->ContainsControllerNode && ContainsControllerNode && (SerialDevices[Index]->Instance == ControllerNumber)) ) @@ -1016,7 +1023,11 @@ SerialControllerDriverStart ( // Restore the PCI attributes when all children is destroyed (PciDeviceInfo->ChildCount == 0). // PciDeviceInfo = AllocatePool (sizeof (PCI_DEVICE_INFO)); - ASSERT (PciDeviceInfo != NULL); + if (PciDeviceInfo == NULL) { + ASSERT (PciDeviceInfo != NULL); + return EFI_OUT_OF_RESOURCES; + } + PciDeviceInfo->ChildCount = 0; PciDeviceInfo->PciIo = ParentIo.PciIo; Status = ParentIo.PciIo->Attributes ( @@ -1047,9 +1058,16 @@ SerialControllerDriverStart ( // // Re-use the PciDeviceInfo stored in existing children. // - ASSERT ((SerialDevices != NULL) && (SerialDevices[0] != NULL)); + if ((SerialDevices == NULL) || (SerialDevices[0] == NULL)) { + ASSERT ((SerialDevices != NULL) && (SerialDevices[0] != NULL)); + return EFI_UNSUPPORTED; + } + PciDeviceInfo = SerialDevices[0]->PciDeviceInfo; - ASSERT (PciDeviceInfo != NULL); + if (PciDeviceInfo == NULL) { + ASSERT (PciDeviceInfo != NULL); + return EFI_UNSUPPORTED; + } } Status = EFI_NOT_FOUND; diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c index 52551a3709..e8cb7ede54 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c @@ -521,7 +521,11 @@ XhcInitSched ( Entries = (Xhc->MaxSlotsEn + 1) * sizeof (UINT64); Dcbaa = UsbHcAllocateMem (Xhc->MemPool, Entries, FALSE); ASSERT (Dcbaa != NULL); - ZeroMem (Dcbaa, Entries); + if (Dcbaa != NULL) { + ZeroMem (Dcbaa, Entries); + } else { + return; + } // // A Scratchpad Buffer is a PAGESIZE block of system memory located on a PAGESIZE boundary. @@ -809,8 +813,13 @@ CreateEventRing ( Size = sizeof (TRB_TEMPLATE) * EVENT_RING_TRB_NUMBER; Buf = UsbHcAllocateMem (Xhc->MemPool, Size, TRUE); - ASSERT (Buf != NULL); + if (Buf == NULL) { + ASSERT (Buf != NULL); + return; + } + ASSERT (((UINTN)Buf & 0x3F) == 0); + ZeroMem (Buf, Size); EventRing->EventRingSeg0 = Buf; @@ -828,8 +837,13 @@ CreateEventRing ( Size = sizeof (EVENT_RING_SEG_TABLE_ENTRY) * ERST_NUMBER; Buf = UsbHcAllocateMem (Xhc->MemPool, Size, FALSE); - ASSERT (Buf != NULL); + if (Buf == NULL) { + ASSERT (Buf != NULL); + return; + } + ASSERT (((UINTN)Buf & 0x3F) == 0); + ZeroMem (Buf, Size); ERSTBase = (EVENT_RING_SEG_TABLE_ENTRY *)Buf; @@ -908,6 +922,10 @@ CreateTransferRing ( Buf = UsbHcAllocateMem (Xhc->MemPool, sizeof (TRB_TEMPLATE) * TrbNum, TRUE); ASSERT (Buf != NULL); ASSERT (((UINTN)Buf & 0x3F) == 0); + if (Buf == NULL) { + return; + } + ZeroMem (Buf, sizeof (TRB_TEMPLATE) * TrbNum); TransferRing->RingSeg0 = Buf; @@ -1041,6 +1059,10 @@ IsTransferRingTrb ( EFI_PHYSICAL_ADDRESS PhyAddr; CheckedTrb = Urb->TrbStart; + if (CheckedTrb == NULL) { + return FALSE; + } + for (Index = 0; Index < Urb->TrbNum; Index++) { if (Trb == CheckedTrb) { return TRUE; @@ -1164,6 +1186,10 @@ XhcCheckUrbResult ( // PhyAddr = (EFI_PHYSICAL_ADDRESS)(EvtTrb->TRBPtrLo | LShiftU64 ((UINT64)EvtTrb->TRBPtrHi, 32)); TRBPtr = (TRB_TEMPLATE *)(UINTN)UsbHcGetHostAddrForPciAddr (Xhc->MemPool, (VOID *)(UINTN)PhyAddr, sizeof (TRB_TEMPLATE), FALSE); + if (TRBPtr == NULL) { + ASSERT (TRBPtr != NULL); + goto EXIT; + } // // Update the status of URB including the pending URB, the URB that is currently checked, @@ -2246,6 +2272,10 @@ XhcInitializeDeviceSlot ( InputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (INPUT_CONTEXT), FALSE); ASSERT (InputContext != NULL); ASSERT (((UINTN)InputContext & 0x3F) == 0); + if (InputContext == NULL) { + return RETURN_OUT_OF_RESOURCES; + } + ZeroMem (InputContext, sizeof (INPUT_CONTEXT)); Xhc->UsbDevContext[SlotId].InputContext = (VOID *)InputContext; @@ -2349,6 +2379,10 @@ XhcInitializeDeviceSlot ( OutputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (DEVICE_CONTEXT), FALSE); ASSERT (OutputContext != NULL); ASSERT (((UINTN)OutputContext & 0x3F) == 0); + if (OutputContext == NULL) { + return EFI_OUT_OF_RESOURCES; + } + ZeroMem (OutputContext, sizeof (DEVICE_CONTEXT)); Xhc->UsbDevContext[SlotId].OutputContext = OutputContext; @@ -2472,6 +2506,10 @@ XhcInitializeDeviceSlot64 ( InputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (INPUT_CONTEXT_64), FALSE); ASSERT (InputContext != NULL); ASSERT (((UINTN)InputContext & 0x3F) == 0); + if (InputContext == NULL) { + return EFI_OUT_OF_RESOURCES; + } + ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64)); Xhc->UsbDevContext[SlotId].InputContext = (VOID *)InputContext; @@ -2575,6 +2613,10 @@ XhcInitializeDeviceSlot64 ( OutputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (DEVICE_CONTEXT_64), FALSE); ASSERT (OutputContext != NULL); ASSERT (((UINTN)OutputContext & 0x3F) == 0); + if (OutputContext == NULL) { + return EFI_OUT_OF_RESOURCES; + } + ZeroMem (OutputContext, sizeof (DEVICE_CONTEXT_64)); Xhc->UsbDevContext[SlotId].OutputContext = OutputContext; diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c index 158749b53c..2c3d1495b9 100644 --- a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c @@ -676,6 +676,9 @@ XhcPeiCheckUrbResult ( // PhyAddr = (EFI_PHYSICAL_ADDRESS)(EvtTrb->TRBPtrLo | LShiftU64 ((UINT64)EvtTrb->TRBPtrHi, 32)); TRBPtr = (TRB_TEMPLATE *)(UINTN)UsbHcGetHostAddrForPciAddr (Xhc->MemPool, (VOID *)(UINTN)PhyAddr, sizeof (TRB_TEMPLATE), FALSE); + if (TRBPtr == NULL) { + return FALSE; + } // // Update the status of Urb according to the finished event regardless of whether @@ -1120,7 +1123,11 @@ XhcPeiInitializeDeviceSlot ( // 1) Allocate an Input Context data structure (6.2.5) and initialize all fields to '0'. // InputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (INPUT_CONTEXT)); - ASSERT (InputContext != NULL); + if (InputContext == NULL) { + ASSERT (InputContext != NULL); + return EFI_OUT_OF_RESOURCES; + } + ASSERT (((UINTN)InputContext & 0x3F) == 0); ZeroMem (InputContext, sizeof (INPUT_CONTEXT)); @@ -1223,7 +1230,11 @@ XhcPeiInitializeDeviceSlot ( // 6) Allocate the Output Device Context data structure (6.2.1) and initialize it to '0'. // OutputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (DEVICE_CONTEXT)); - ASSERT (OutputContext != NULL); + if (OutputContext == NULL) { + ASSERT (OutputContext != NULL); + return EFI_OUT_OF_RESOURCES; + } + ASSERT (((UINTN)OutputContext & 0x3F) == 0); ZeroMem (OutputContext, sizeof (DEVICE_CONTEXT)); @@ -1335,7 +1346,11 @@ XhcPeiInitializeDeviceSlot64 ( // 1) Allocate an Input Context data structure (6.2.5) and initialize all fields to '0'. // InputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (INPUT_CONTEXT_64)); - ASSERT (InputContext != NULL); + if (InputContext == NULL) { + ASSERT (InputContext != NULL); + return EFI_OUT_OF_RESOURCES; + } + ASSERT (((UINTN)InputContext & 0x3F) == 0); ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64)); @@ -1438,7 +1453,11 @@ XhcPeiInitializeDeviceSlot64 ( // 6) Allocate the Output Device Context data structure (6.2.1) and initialize it to '0'. // OutputContext = UsbHcAllocateMem (Xhc->MemPool, sizeof (DEVICE_CONTEXT_64)); - ASSERT (OutputContext != NULL); + if (OutputContext == NULL) { + ASSERT (OutputContext != NULL); + return EFI_OUT_OF_RESOURCES; + } + ASSERT (((UINTN)OutputContext & 0x3F) == 0); ZeroMem (OutputContext, sizeof (DEVICE_CONTEXT_64)); @@ -2682,7 +2701,11 @@ XhcPeiCreateEventRing ( Size = sizeof (TRB_TEMPLATE) * EVENT_RING_TRB_NUMBER; Buf = UsbHcAllocateMem (Xhc->MemPool, Size); - ASSERT (Buf != NULL); + if (Buf == NULL ) { + ASSERT (Buf != NULL); + return; + } + ASSERT (((UINTN)Buf & 0x3F) == 0); ZeroMem (Buf, Size); @@ -2701,7 +2724,11 @@ XhcPeiCreateEventRing ( Size = sizeof (EVENT_RING_SEG_TABLE_ENTRY) * ERST_NUMBER; Buf = UsbHcAllocateMem (Xhc->MemPool, Size); - ASSERT (Buf != NULL); + if (Buf == NULL) { + ASSERT (Buf != NULL); + return; + } + ASSERT (((UINTN)Buf & 0x3F) == 0); ZeroMem (Buf, Size); @@ -2918,7 +2945,10 @@ XhcPeiInitSched ( // Size = (Xhc->MaxSlotsEn + 1) * sizeof (UINT64); Dcbaa = UsbHcAllocateMem (Xhc->MemPool, Size); - ASSERT (Dcbaa != NULL); + if (Dcbaa == NULL) { + ASSERT (Dcbaa != NULL); + return; + } // // A Scratchpad Buffer is a PAGESIZE block of system memory located on a PAGESIZE boundary.