[edk2-devel] [PATCH v1 0/2] BZ 4221: Fix MdeModulePkg/Bus/Pci/XhciDxe issues pointed by Coverity

[edk2-devel] [PATCH v1 0/2] BZ 4221: Fix MdeModulePkg/Bus/Pci/XhciDxe issues pointed by Coverity

[Ranbir Singh]

Ranbir Singh (2):
  MdeModulePkg/Bus/Pci/XhciDxe: Fix FORWARD_NULL Coverity issues
  MdeModulePkg/Bus/Pci/XhciDxe: Fix MISSING_BREAK Coverity issues

 MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c  | 14 ++++++++++++++
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c |  6 ++++++
 2 files changed, 20 insertions(+)

-- 
2.34.1



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

[edk2-devel] [PATCH v1 1/2] MdeModulePkg/Bus/Pci/XhciDxe: Fix FORWARD_NULL Coverity issues

[Ranbir Singh]

From: Ranbir Singh <Ranbir.Singh3@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: 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/XhciDxe/UsbHcMem.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
index b54187ec228e..b0654f148c4f 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
@@ -267,6 +267,11 @@ UsbHcGetPciAddrForHostAddr (
   }
 
   ASSERT ((Block != NULL));
+
+  if (Block == NULL) {
+    return 0;
+  }
+
   //
   // calculate the pci memory address for host memory address.
   //
@@ -322,6 +327,11 @@ UsbHcGetHostAddrForPciAddr (
   }
 
   ASSERT ((Block != NULL));
+
+  if (Block == NULL) {
+    return 0;
+  }
+
   //
   // calculate the pci memory address for host memory address.
   //
@@ -603,6 +613,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 (#109273): https://edk2.groups.io/g/devel/message/109273
Mute This Topic: https://groups.io/mt/101729056/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 2/2] MdeModulePkg/Bus/Pci/XhciDxe: Fix MISSING_BREAK Coverity issues

[Ranbir Singh]

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

The functions
XhcInitializeEndpointContext and XhcInitializeEndpointContext64 has
a switch-case code in which the case USB_ENDPOINT_CONTROL: falls
through to default:

While this may be intentional, it is not evident to any general code
reader as well as any static analyzer tool. Just adding

    // No break; here as this is an intentional fallthrough.

as comment in between makes any reader as well as Coverity happy.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4221

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/XhciDxe/XhciSched.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 05528a478baf..2afecd40cab0 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2979,6 +2979,9 @@ XhcInitializeEndpointContext (
         // Do not support control transfer now.
         //
         DEBUG ((DEBUG_INFO, "XhcInitializeEndpointContext: Unsupport Control EP found, Transfer ring is not allocated.\n"));
+        //
+        // No break; here as this is an intentional fall through.
+        //
       default:
         DEBUG ((DEBUG_INFO, "XhcInitializeEndpointContext: Unknown EP found, Transfer ring is not allocated.\n"));
         EpDesc = (USB_ENDPOINT_DESCRIPTOR *)((UINTN)EpDesc + EpDesc->Length);
@@ -3182,6 +3185,9 @@ XhcInitializeEndpointContext64 (
         // Do not support control transfer now.
         //
         DEBUG ((DEBUG_INFO, "XhcInitializeEndpointContext64: Unsupport Control EP found, Transfer ring is not allocated.\n"));
+        //
+        // No break; here as this is an intentional fall through.
+        //
       default:
         DEBUG ((DEBUG_INFO, "XhcInitializeEndpointContext64: Unknown EP found, Transfer ring is not allocated.\n"));
         EpDesc = (USB_ENDPOINT_DESCRIPTOR *)((UINTN)EpDesc + EpDesc->Length);
-- 
2.34.1



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