From: Ranbir Singh <Ranbir.Singh3@Dell.com>

The function UsbHcGetPciAddressForHostMem has

  ASSERT ((Block != NULL)); and

and the function UsbHcFreeMem has

  ASSERT (Block != NULL);

statement 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=4211

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Veeresh Sangolli <veeresh.sangolli@dellteam.com>
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
---
 MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c b/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
index c3d46f60bed5..3794f888e132 100644
--- a/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
@@ -250,6 +250,11 @@ UsbHcGetPciAddressForHostMem (
   }
 
   ASSERT ((Block != NULL));
+
+  if (Block == NULL) {
+    return 0;
+  }
+
   //
   // calculate the pci memory address for host memory address.
   //
@@ -536,6 +541,10 @@ UsbHcFreeMem (
   //
   ASSERT (Block != NULL);
 
+  if (Block == NULL) {
+    return;
+  }
+
   //
   // Release the current memory block if it is empty and not the head
   //
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106995): https://edk2.groups.io/g/devel/message/106995
Mute This Topic: https://groups.io/mt/100212110/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: Ranbir Singh <rsingh@ventanamicro.com>
> Sent: Monday, July 17, 2023 7:39 PM
> To: devel@edk2.groups.io; rsingh@ventanamicro.com
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Veeresh
> Sangolli <veeresh.sangolli@dellteam.com>
> Subject: [PATCH v1 2/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix
> FORWARD_NULL Coverity issues
> 
> From: Ranbir Singh <Ranbir.Singh3@Dell.com>
> 
> The function UsbHcGetPciAddressForHostMem has
> 
>   ASSERT ((Block != NULL)); and
> 
> and the function UsbHcFreeMem has
> 
>   ASSERT (Block != NULL);
> 
> statement 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=4211
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Co-authored-by: Veeresh Sangolli <veeresh.sangolli@dellteam.com>
> Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
> Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
> ---
>  MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
> b/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
> index c3d46f60bed5..3794f888e132 100644
> --- a/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
> +++ b/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
> @@ -250,6 +250,11 @@ UsbHcGetPciAddressForHostMem (
>    }
> 
> 
> 
>    ASSERT ((Block != NULL));
> 
> +
> 
> +  if (Block == NULL) {
> 
> +    return 0;
> 
> +  }
> 
> +
> 
>    //
> 
>    // calculate the pci memory address for host memory address.
> 
>    //
> 
> @@ -536,6 +541,10 @@ UsbHcFreeMem (
>    //
> 
>    ASSERT (Block != NULL);
> 
> 
> 
> +  if (Block == NULL) {
> 
> +    return;
> 
> +  }
> 
> +
> 
>    //
> 
>    // Release the current memory block if it is empty and not the head
> 
>    //
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107680): https://edk2.groups.io/g/devel/message/107680
Mute This Topic: https://groups.io/mt/100212110/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-