public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ranbir Singh" <Ranbir.Singh3@Dell.com>
To: devel@edk2.groups.io
Subject: [PATCH] MdeModulePkg/Bus/Pci/PciHostBridgeDxe: Fix various Coverity issues
Date: Tue, 03 Jan 2023 22:49:13 -0800	[thread overview]
Message-ID: <KtPU.1672814953269160475.4ifo@groups.io> (raw)

[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]

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 >= 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=4212
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/MdeModulePkg/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);
+
+            if (Index >= TypeMax) {
+                continue;
+            }
+
ResNodeHandled[Index] = TRUE;
Alignment             = RootBridge->ResAllocNode[Index].Alignment;
BitsOfAlignment       = LowBitSet64 (Alignment + 1);
@@ -1526,6 +1531,10 @@ SubmitResources (
return EFI_INVALID_PARAMETER;
}

+            //
+            // No break; here as this is 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

[-- Attachment #2: Type: text/html, Size: 3750 bytes --]

                 reply	other threads:[~2023-01-04  6:49 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=KtPU.1672814953269160475.4ifo@groups.io \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox