[edk2-devel] [PATCH v2 0/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix issues pointed by Coverity

[edk2-devel] [PATCH v2 0/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix issues pointed by Coverity

[Ranbir Singh]

v1 -> v2:
  - Update patch wrt BAD_SHIFT changes as per review comments

Ranbir Singh (2):
  MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT Coverity issue
  MdeModulePkg/Bus/Pci/UhciDxe: Fix FORWARD_NULL Coverity issues

 MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 10 +++++++++-
 MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c  |  9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107788): https://edk2.groups.io/g/devel/message/107788
Mute This Topic: https://groups.io/mt/100774223/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT Coverity issue

[Ranbir Singh]

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

The function UhciConvertPollRate has a check

    ASSERT (Interval != 0);

but this comes into play only in DEBUG mode. In Release mode, there is
no handling if the Interval parameter value is ZERO. To avoid shifting
by a negative amount later in the code flow in this undesirable case,
it is better to handle it as well by treating it same as if 1 is sent.

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/UhciSched.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
index c08f9496969b..408e7d5ab7f3 100644
--- a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
+++ b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
@@ -197,7 +197,7 @@ UhciDestoryFrameList (
 }
 
 /**
-  Convert the poll rate to the maxium 2^n that is smaller
+  Convert the poll rate to the maximum 2^n that is smaller
   than Interval.
 
   @param  Interval               The poll rate to convert.
@@ -214,6 +214,14 @@ UhciConvertPollRate (
 
   ASSERT (Interval != 0);
 
+  //
+  // To safeguard RELEASE mode wherein ASSERT is effectively not there,
+  // if inadvertently Interval is still 0 here, treat it the same as 1.
+  //
+  if (Interval == 0) {
+    Interval = 1;
+  }
+
   //
   // Find the index (1 based) of the highest non-zero bit
   //
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107789): https://edk2.groups.io/g/devel/message/107789
Mute This Topic: https://groups.io/mt/100774224/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT Coverity issue

[Wu, Hao A]

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

Best Regards,
Hao Wu

> -----Original Message-----
> From: Ranbir Singh <rsingh@ventanamicro.com>
> Sent: Wednesday, August 16, 2023 1:38 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 v2 1/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT
> Coverity issue
> 
> From: Ranbir Singh <Ranbir.Singh3@Dell.com>
> 
> The function UhciConvertPollRate has a check
> 
>     ASSERT (Interval != 0);
> 
> but this comes into play only in DEBUG mode. In Release mode, there is
> no handling if the Interval parameter value is ZERO. To avoid shifting
> by a negative amount later in the code flow in this undesirable case,
> it is better to handle it as well by treating it same as if 1 is sent.
> 
> 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/UhciSched.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> index c08f9496969b..408e7d5ab7f3 100644
> --- a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> @@ -197,7 +197,7 @@ UhciDestoryFrameList (
>  }
> 
> 
> 
>  /**
> 
> -  Convert the poll rate to the maxium 2^n that is smaller
> 
> +  Convert the poll rate to the maximum 2^n that is smaller
> 
>    than Interval.
> 
> 
> 
>    @param  Interval               The poll rate to convert.
> 
> @@ -214,6 +214,14 @@ UhciConvertPollRate (
> 
> 
>    ASSERT (Interval != 0);
> 
> 
> 
> +  //
> 
> +  // To safeguard RELEASE mode wherein ASSERT is effectively not there,
> 
> +  // if inadvertently Interval is still 0 here, treat it the same as 1.
> 
> +  //
> 
> +  if (Interval == 0) {
> 
> +    Interval = 1;
> 
> +  }
> 
> +
> 
>    //
> 
>    // Find the index (1 based) of the highest non-zero bit
> 
>    //
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108896): https://edk2.groups.io/g/devel/message/108896
Mute This Topic: https://groups.io/mt/100774224/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix FORWARD_NULL Coverity issues

[Ranbir Singh]

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>
Reviewed-by: Hao A Wu <hao.a.wu@intel.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 (#107790): https://edk2.groups.io/g/devel/message/107790
Mute This Topic: https://groups.io/mt/100774225/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix issues pointed by Coverity

[Wu, Hao A]

Series pushed via:
PR - https://github.com/tianocore/edk2/pull/4856
Commits:
https://github.com/tianocore/edk2/commit/e9f5d8c0e066da55b3f79dfdbf4df5fc97ca5916
https://github.com/tianocore/edk2/commit/28a267af4024c329e58121ccd9bf5f4f7aabc0f4

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ranbir
> Singh
> Sent: Wednesday, August 16, 2023 1:38 PM
> To: devel@edk2.groups.io; rsingh@ventanamicro.com
> Subject: [edk2-devel] [PATCH v2 0/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix
> issues pointed by Coverity
> 
> v1 -> v2:
>   - Update patch wrt BAD_SHIFT changes as per review comments
> 
> Ranbir Singh (2):
>   MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT Coverity issue
>   MdeModulePkg/Bus/Pci/UhciDxe: Fix FORWARD_NULL Coverity issues
> 
>  MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 10 +++++++++-
>  MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c  |  9 +++++++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> --
> 2.34.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108977): https://edk2.groups.io/g/devel/message/108977
Mute This Topic: https://groups.io/mt/100774223/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-