From nobody Sat May 18 20:15:29 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+97916+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+97916+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1672814708; cv=none; d=zohomail.com; s=zohoarc; b=EdrSrJpepjgxVZ4r1NGTBBuwBdQQrxtXCztrnzoEGGc0c/orxngeE/d4yrm1DTxlLLvOccPZivKCxlA5uqz7dEYAIH2WaIHiwcxRX8KHEtjY0E72Kmy5kIo9blu4DvqI+t2wrA5/GzhYbgZtMGXPFeq/WTQO3pj7ksbOgTUIgqs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1672814708; h=Content-Type:Date:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=OuKb52nrCj7TmlVVE69YhaSgre3KUDN8m3IDi4CWnok=; b=jtcNs+FdFRg+i3Rmk3ElFhrtY4NzLzz/Yo4zNKdLBeb4JOqXi2SCvBPG83yrWPMdh7OLVixMaElJ5Ui6exhhX3+bSCF30qGpYH1Jj+Yt/F2bMqertiINOpX0XZ6kyMnuF0GhYpOVaQ8nRZ6zciF3IWbz72utscfA6hBG103bCJ8= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+97916+1787277+3901457@groups.io Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1672814707893599.2976430174406; Tue, 3 Jan 2023 22:45:07 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id qQ65YY1788612xACLZWsHUsW; Tue, 03 Jan 2023 22:45:07 -0800 Subject: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/UhciDxe: Fix various Coverity issues To: devel@edk2.groups.io From: "Ranbir Singh via groups.io" X-Originating-Location: Bengaluru, Karnataka, IN (122.172.85.38) X-Originating-Platform: Windows Chrome 108 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Tue, 03 Jan 2023 22:45:07 -0800 Message-ID: Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,Ranbir.Singh3@Dell.com X-Gm-Message-State: icbKnpV5OcXBoHk7jPT17Ayfx1787277AA= Content-Type: multipart/alternative; boundary="pM1Po69zWQqcHtwAC16w" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1672814707; bh=VmaLvXMPCdJKcj1X4otRs8qPDMGSGQVHJYuuuAetcM4=; h=Content-Type:Date:From:Reply-To:Subject:To; b=HyA5NNq4TKxzF9n0DgvjD8auFQUfeJrLnbX7fP9JJlN8WaErrIQnhi01kvvQdxzgQyz uENHXplOHQ6zq0P0fPjssGZ/uq3v+6wNp19QuBXsBeGdoNnesZbQfaoyTMylT160oRHVL SbtRJlf3DWTVAyWgyLpo71zpWmwIRZfFvZ8= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1672814709205100001 --pM1Po69zWQqcHtwAC16w Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 The function UhciConvertPollRate has a check ASSERT (Interval !=3D 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 !=3D 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 !=3D 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=3D4211 Signed-off-by: Ranbir Singh --- MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 4 ++++ MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c=C2=A0 | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c b/MdeModulePkg/Bus/Pc= i/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 !=3D 0); +=C2=A0 if (Interval =3D=3D 0) { +=C2=A0 =C2=A0 return 0; +=C2=A0 } + // // 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 !=3D NULL)); + +=C2=A0 if (Block =3D=3D NULL) { +=C2=A0 =C2=A0 return 0; +=C2=A0 } + // // calculate the pci memory address for host memory address. // @@ -536,6 +541,10 @@ UsbHcFreeMem ( // ASSERT (Block !=3D NULL); +=C2=A0 if (Block =3D=3D NULL) { +=C2=A0 =C2=A0 return; +=C2=A0 } + // // Release the current memory block if it is empty and not the head // -- 2.36.1.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --pM1Po69zWQqcHtwAC16w Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable
The function UhciConvertPollRate has a check
 
    ASSERT (Interval !=3D 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,<= /div>
it is better to handle it as well by simply returning ZERO.
 
The functions UsbHcGetPciAddressForHostMem and UsbHcFreeMem do have
 
    ASSERT ((Block !=3D 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<= /div>
loop and the loop exits because of Block !=3D NULL; condition, then th= ere
is no "Block" NULL pointer check afterwards and the code proceeds to d= o
dereferencing "Block" which will lead to CRASH.
 
Hence, for safety add NULL pointer checks always.
 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4211
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/B= us/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 !=3D 0);
 
+  if (Interval =3D=3D 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/Bu= s/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 !=3D NULL));
+
+  if (Block =3D=3D NULL) {
+    return 0;
+  }
+
   //
   // calculate the pci memory address for host memory addre= ss.
   //
@@ -536,6 +541,10 @@ UsbHcFreeMem (
   //
   ASSERT (Block !=3D NULL);
 
+  if (Block =3D=3D NULL) {
+    return;
+  }
+
   //
   // Release the current memory block if it is empty and no= t the head
   //
--
2.36.1.windows.1
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

= View/Reply Online (#97916) | =20 | Mute = This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [importer@patchew.org]

_._,_._,_
--pM1Po69zWQqcHtwAC16w--