public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues
@ 2023-07-03 11:44 Ranbir Singh
  2023-07-12  4:48 ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Ranbir Singh @ 2023-07-03 11:44 UTC (permalink / raw)
  To: devel, rsingh; +Cc: Hao A Wu, Ray Ni

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

The function UsbHcGetPciAddressForHostMem has

    ASSERT ((Block != NULL));

and the 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 the
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.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4210
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
---
 MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c b/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
index 0a3ceb9f711a..79575b6f6304 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues
  2023-07-03 11:44 [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues Ranbir Singh
@ 2023-07-12  4:48 ` Wu, Hao A
  2023-07-17  2:25   ` [edk2-devel] " Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2023-07-12  4:48 UTC (permalink / raw)
  To: Ranbir Singh, devel@edk2.groups.io; +Cc: Ni, Ray

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 3, 2023 7:44 PM
> To: devel@edk2.groups.io; rsingh@ventanamicro.com
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL
> Coverity issues
> 
> From: Ranbir Singh <Ranbir.Singh3@Dell.com>
> 
> The function UsbHcGetPciAddressForHostMem has
> 
>     ASSERT ((Block != NULL));
> 
> and the 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 the
> 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.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4210
> Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
> Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
> ---
>  MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> index 0a3ceb9f711a..79575b6f6304 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues
  2023-07-12  4:48 ` Wu, Hao A
@ 2023-07-17  2:25   ` Wu, Hao A
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2023-07-17  2:25 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wu, Hao A, Ranbir Singh; +Cc: Ni, Ray

Merged via:
PR - https://github.com/tianocore/edk2/pull/4648
Commit - https://github.com/tianocore/edk2/commit/dd49d448b0815c65847241a3faa957e3b4605001

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A
> Sent: Wednesday, July 12, 2023 12:49 PM
> To: Ranbir Singh <rsingh@ventanamicro.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix
> FORWARD_NULL Coverity issues
> 
> 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 3, 2023 7:44 PM
> > To: devel@edk2.groups.io; rsingh@ventanamicro.com
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL
> > Coverity issues
> >
> > From: Ranbir Singh <Ranbir.Singh3@Dell.com>
> >
> > The function UsbHcGetPciAddressForHostMem has
> >
> >     ASSERT ((Block != NULL));
> >
> > and the 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 the
> > 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.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4210
> > Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
> > Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
> > ---
> >  MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> > b/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> > index 0a3ceb9f711a..79575b6f6304 100644
> > --- a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> > +++ b/MdeModulePkg/Bus/Pci/EhciDxe/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
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-07-17  2:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-03 11:44 [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues Ranbir Singh
2023-07-12  4:48 ` Wu, Hao A
2023-07-17  2:25   ` [edk2-devel] " Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox