From nobody Sun May 19 03:12:39 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+97917+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+97917+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1672814954; cv=none; d=zohomail.com; s=zohoarc; b=UrvENRp1i3x2RGgPqyx1GR4PV+2Wyq3im1+HZo+WKpc584zZ3uT4+bn06mxQ4O94Tu5PGSOGmIgjZ35nnKdDFMfugB/ijkIXoMBB75XJfBuhnRiMYKa1umRBXlQ8rwc6fd4Tyud6NosOyZMdgIK+8vn9TeCRf41OqPNynJSZpZw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1672814954; h=Content-Type:Date:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=XEwv+6lTx/NBXCFDRcdg/B+Jixo/mP+iHwDdIgKa7CM=; b=VNpM8NBYlLUtZr68RBmv1497Y4adEZA1DU2sPMipKliQr7foDN8s8Lxa56bHnGfV3IWXNtupV07u1mfpa3MwtIR6NukELwMmXJtj8CmVJw7xqmTKmYvhd0icOUO5F/dODdhkF/X4m3rc7BMVDM2ugPQjzopqJ34PuFutfK2lG4I= 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+97917+1787277+3901457@groups.io Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1672814954045674.437356808296; Tue, 3 Jan 2023 22:49:14 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id MvILYY1788612xWsBQNqAPoe; Tue, 03 Jan 2023 22:49:13 -0800 Subject: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/PciHostBridgeDxe: 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:49:13 -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: WmTmtVm6KszcExOOK1pfhKLcx1787277AA= Content-Type: multipart/alternative; boundary="WNAWtpsQ7KXsg3vunyxR" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1672814953; bh=KH/e4qj9jUVL3sC6k2c3m+dr7lddtqiOkSl4bkOk+0c=; h=Content-Type:Date:From:Reply-To:Subject:To; b=nvXCCywSm4Z2iMNyJwh8nhAzhqfLEgmomTbJ0uopOtHOh3fDIqHPMypvRNrXyA+5HPV ir3gULrrbW1U+PzzEnJ55zJHqwl8F6ixtTFq+ZnPiLznk5DVI3tC4KZ+qi/6dKpROy5Nd SjUjx+ie5yPbmhENpSziniqA/prSahGRp5g= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1672814954416100002 --WNAWtpsQ7KXsg3vunyxR Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 The function NotifyPhase has a check ASSERT (Index < TypeMax); but this comes into play only in DEBUG mode. In Release mode, there is no handling if the Index value is within array limits or not. If for whatever reasons, the Index does not get re-assigned to Index2 at line 137, then it remains at TypeMax as assigned earlier at line 929. This poses array overrun risk at lines 942 and 943. It is better to deploy a safety check before line 942 as if (Index >=3D TypeMax) { continue; } The function SubmitResources has a switch-case code in which the case ACPI_ADDRESS_SPACE_TYPE_MEM: which falls through to case ACPI_ADDRESS_SPACE_TYPE_IO: if there is no scenario of return EFI_INVALID_PARAMETER; 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=3D4212 Signed-off-by: Ranbir Singh --- MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeMod= ulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c index b20bcd310a..83f1ad450f 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c @@ -939,6 +939,11 @@ NotifyPhase ( } ASSERT (Index < TypeMax); + +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (Index >=3D TypeMax) { +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue; +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } + ResNodeHandled[Index] =3D TRUE; Alignment=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D RootBridge->Re= sAllocNode[Index].Alignment; BitsOfAlignment=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D LowBitSet64 (Alignment + 1); @@ -1526,6 +1531,10 @@ SubmitResources ( return EFI_INVALID_PARAMETER; } +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // No break; here as this is an = intentional fall through. +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // + case ACPI_ADDRESS_SPACE_TYPE_IO: // // Check aligment, it should be of the form 2^n-1 -- 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 (#97917): https://edk2.groups.io/g/devel/message/97917 Mute This Topic: https://groups.io/mt/96046026/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- --WNAWtpsQ7KXsg3vunyxR Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable
The function NotifyPhase has a check
 
    ASSERT (Index < TypeMax);
 
but this comes into play only in DEBUG mode. In Release mode, there is=
no handling if the Index value is within array limits or not. If for
whatever reasons, the Index does not get re-assigned to Index2 at line=
137, then it remains at TypeMax as assigned earlier at line 929. This<= /div>
poses array overrun risk at lines 942 and 943. It is better to deploy<= /div>
a safety check before line 942 as
 
            if (Index >=3D TypeMax) {=
                continue;
            }
 
The function SubmitResources has a switch-case code in which the
case ACPI_ADDRESS_SPACE_TYPE_MEM: which falls through to
case ACPI_ADDRESS_SPACE_TYPE_IO: if there is no scenario of
return EFI_INVALID_PARAMETER;
 
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=3D4212
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 9 ++++++= +++
 1 file changed, 9 insertions(+)
 
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/M= deModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index b20bcd310a..83f1ad450f 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -939,6 +939,11 @@ NotifyPhase (
             }
 
             ASSERT (Index < Typ= eMax);
+
+            if (Index >=3D TypeMax) = {
+                continue;
+            }
+
             ResNodeHandled[Index] = =3D TRUE;
             Alignment   =          =3D RootBridge->ResAllocNode[Index].A= lignment;
             BitsOfAlignment  =      =3D LowBitSet64 (Alignment + 1);
@@ -1526,6 +1531,10 @@ SubmitResources (
               return EFI_INVA= LID_PARAMETER;
             }
 
+            //
+            // No break; here as this i= s an intentional fall through.
+            //
+
           case ACPI_ADDRESS_SPACE_TYPE_= IO:
             //
             // Check aligment, it = should be of the form 2^n-1
--
2.36.1.windows.1
_._,_._,_
Groups.io Links:

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

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

_._,_._,_
--WNAWtpsQ7KXsg3vunyxR--