[edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/UhciDxe: Fix various Coverity issues

Ranbir Singh via groups.io posted 1 patch 1 year, 4 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 4 ++++
MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c  | 9 +++++++++
2 files changed, 13 insertions(+)
[edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/UhciDxe: Fix various Coverity issues
Posted by Ranbir Singh via groups.io 1 year, 4 months ago
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 simply returning ZERO.

The functions UsbHcGetPciAddressForHostMem 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=4211
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
---
MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 4 ++++
MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c  | 9 +++++++++
2 files changed, 13 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
index c08f949696..8ddef4b68c 100644
--- a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
+++ b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
@@ -214,6 +214,10 @@ UhciConvertPollRate (

ASSERT (Interval != 0);

+  if (Interval == 0) {
+    return 0;
+  }
+
//
// Find the index (1 based) of the highest non-zero bit
//
diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c b/MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c
index c3d46f60be..3794f888e1 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.36.1.windows.1


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