On 11/10/23 06:22, Ranbir Singh wrote: > From: Ranbir Singh <ranbir.sin...@dell.com> > > The functions UsbHcGetHostAddrForPciAddr, UsbHcGetPciAddrForHostAddr > and UsbHcFreeMem do have > > ASSERT ((Block != NULL)); > > statements after for loop, but these are applicable only in DEBUG mode. > In RELEASE mode, if for whatever reasons there is no match inside for > loop and the loop exits because of Block != NULL; condition, then there > is no "Block" NULL pointer check afterwards and the code proceeds to do > dereferencing "Block" which will lead to CRASH. > > Hence, for safety add NULL pointer checks always. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4221 > > 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/XhciDxe/UsbHcMem.c | 29 ++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c > b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c > index b54187ec228e..597cbe4646e8 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c > @@ -267,6 +267,16 @@ UsbHcGetPciAddrForHostAddr ( > } > > ASSERT ((Block != NULL)); > + > + if (Block == NULL) { > + // > + // Should never be here > + // > + DEBUG ((DEBUG_ERROR, "UsbHcGetPciAddrForHostAddr: Invalid host memory > pointer passed\n")); > + CpuDeadLoop (); > + return 0; > + } > + > // > // calculate the pci memory address for host memory address. > // > @@ -322,6 +332,16 @@ UsbHcGetHostAddrForPciAddr ( > } > > ASSERT ((Block != NULL)); > + > + if (Block == NULL) { > + // > + // Should never be here > + // > + DEBUG ((DEBUG_ERROR, "UsbHcGetHostAddrForPciAddr: Invalid pci memory > pointer passed\n")); > + CpuDeadLoop (); > + return 0; > + } > + > // > // calculate the pci memory address for host memory address. > // > @@ -603,6 +623,15 @@ UsbHcFreeMem ( > // > ASSERT (Block != NULL); > > + if (Block == NULL) { > + // > + // Should never be here > + // > + DEBUG ((DEBUG_ERROR, "UsbHcFreeMem: Invalid memory pointer passed\n")); > + CpuDeadLoop (); > + return; > + } > + > // > // Release the current memory block if it is empty and not the head > //
Reviewed-by: Laszlo Ersek <ler...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111166): https://edk2.groups.io/g/devel/message/111166 Mute This Topic: https://groups.io/mt/102502055/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
