On 11/7/23 07:19, Ranbir Singh wrote: > From: Ranbir Singh <ranbir.sin...@dell.com> > > The function StartPciDevices has a check > > ASSERT (RootBridge != NULL); > > but this comes into play only in DEBUG mode. In Release mode, there > is no handling if the RootBridge value is NULL and the code proceeds > to unconditionally dereference "RootBridge" which will lead to CRASH. > > Hence, for safety add NULL pointer checks always and return > EFI_NOT_READY if RootBridge value is NULL which is one of the return > values as mentioned in the function description header. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4239 > > Cc: Ray Ni <ray...@intel.com> > Co-authored-by: Veeresh Sangolli <veeresh.sango...@dellteam.com> > Signed-off-by: Ranbir Singh <ranbir.sin...@dell.com> > Signed-off-by: Ranbir Singh <rsi...@ventanamicro.com> > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > index 581e9075ad41..3de80d98370e 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > @@ -772,7 +772,10 @@ StartPciDevices ( > LIST_ENTRY *CurrentLink; > > RootBridge = GetRootBridgeByHandle (Controller); > - ASSERT (RootBridge != NULL); > + if (RootBridge == NULL) { > + return EFI_NOT_READY; > + } > + > ThisHostBridge = RootBridge->PciRootBridgeIo->ParentHandle; > > CurrentLink = mPciDevicePool.ForwardLink;
I don't think this is a good fix. There is one call site, namely in PciBusDriverBindingStart(). That call site does not check the return value. (Of course /s) I think that this ASSERT() can indeed never fail. Therefore I suggest CpuDeadLoop() instead. If you insist that CpuDeadLoop() is "too risky" here, then the patch is acceptable, but then the StartPciDevices() call site in PciBusDriverBindingStart() must check the error properly: we must not install "gEfiPciEnumerationCompleteProtocolGuid", and the function must propagate the error outwards. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110865): https://edk2.groups.io/g/devel/message/110865 Mute This Topic: https://groups.io/mt/102438320/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
