public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Krzysztof Koch" <krzysztof.koch@arm.com>
To: <devel@edk2.groups.io>
Cc: <jaben.carsey@intel.com>, <ray.ni@intel.com>,
	<zhichao.gao@intel.com>, <Sami.Mujawar@arm.com>,
	<Matteo.Carlini@arm.com>, <nd@arm.com>
Subject: [PATCH v1 5/6] ShellPkg: acpiview: MADT: Split structure length validation
Date: Thu, 18 Jul 2019 13:31:41 +0100	[thread overview]
Message-ID: <20190718123142.5696-6-krzysztof.koch@arm.com> (raw)
In-Reply-To: <20190718123142.5696-1-krzysztof.koch@arm.com>

Split the Interrupt Controller Structure length validation in the
acpiview UEFI shell tool into two logical parts:
1. Ensuring MADT table parser forward progress.
2. Preventing MADT table buffer overruns.

Also, make the condition for infinite loop detection applicable to
all types of Interrupt Controller Structures (for all interrupt models
which can be represented in MADT). Check if the controller length
specified is shorter than the byte size of the first two fields
('Type' and 'Length') present in every valid Interrupt Controller
Structure.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - split MADT structure length validation [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 30 ++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 59c3df0cc8a080497b517baf36fc63f1e4ab866f..52b71f37a40733de2029373306658ca08c78c42d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -290,16 +290,30 @@ ParseAcpiMadt (
       PARSER_PARAMS (MadtInterruptControllerHeaderParser)
       );
 
-    if (((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength) ||
-        (*MadtInterruptControllerLength < 4)) {
+    // Make sure forward progress is made.
+    if (*MadtInterruptControllerLength < 2) {
       IncrementErrorCount ();
       Print (
-         L"ERROR: Invalid Interrupt Controller Length,"
-          L" Type = %d, Length = %d\n",
-         *MadtInterruptControllerType,
-         *MadtInterruptControllerLength
-         );
-      break;
+        L"ERROR: Structure length is too small: " \
+          L"MadtInterruptControllerLength = %d. " \
+          L"MadtInterruptControllerType = %d. MADT parsing aborted.\n",
+        *MadtInterruptControllerLength,
+        *MadtInterruptControllerType
+        );
+      return;
+    }
+
+    // Make sure the MADT structure lies inside the table
+    if ((Offset + *MadtInterruptControllerLength) > AcpiTableLength) {
+      IncrementErrorCount ();
+      Print (
+        L"ERROR: Invalid MADT structure length. " \
+          L"MadtInterruptControllerLength = %d. " \
+          L"RemainingTableBufferLength = %d. MADT parsing aborted.\n",
+        *MadtInterruptControllerLength,
+        AcpiTableLength - Offset
+        );
+      return;
     }
 
     switch (*MadtInterruptControllerType) {
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



  parent reply	other threads:[~2019-07-18 12:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-18 12:31 [PATCH v1 0/6] Acpiview table parsers code style enhancements and refactoring Krzysztof Koch
2019-07-18 12:31 ` [PATCH v1 1/6] ShellPkg: acpiview: Allow passing buffer length to DumpGasStruct() Krzysztof Koch
2019-07-18 15:32   ` [edk2-devel] " Alexei Fedorov
2019-07-19  1:14   ` Gao, Zhichao
     [not found]     ` <VE1PR08MB47831B4BE6619A6C85953A6E84CB0@VE1PR08MB4783.eurprd08.prod.outlook.com>
     [not found]       ` <3CE959C139B4C44DBEA1810E3AA6F9000B808FFA@SHSMSX101.ccr.corp.intel.com>
2019-07-19 10:40         ` Krzysztof Koch
2019-07-22  0:38           ` Gao, Zhichao
2019-07-18 12:31 ` [PATCH v1 2/6] ShellPkg: acpiview: XSDT: Remove redundant ParseAcpi() call Krzysztof Koch
2019-07-18 15:31   ` [edk2-devel] " Alexei Fedorov
2019-07-18 12:31 ` [PATCH v1 3/6] ShellPkg: acpiview: RSDP: Make code consistent with other parsers Krzysztof Koch
2019-07-18 15:31   ` [edk2-devel] " Alexei Fedorov
2019-07-18 12:31 ` [PATCH v1 4/6] ShellPkg: acpiview: SRAT: Minor code style enhancements Krzysztof Koch
2019-07-18 15:31   ` [edk2-devel] " Alexei Fedorov
2019-07-18 12:31 ` Krzysztof Koch [this message]
2019-07-18 15:31   ` [edk2-devel] [PATCH v1 5/6] ShellPkg: acpiview: MADT: Split structure length validation Alexei Fedorov
2019-07-18 12:31 ` [PATCH v1 6/6] ShellPkg: acpiview: IORT: Refactor PMCG node mapping count validation Krzysztof Koch
2019-07-18 15:30   ` [edk2-devel] " Alexei Fedorov
2019-07-18 12:58 ` [PATCH v1 0/6] Acpiview table parsers code style enhancements and refactoring Sami Mujawar
2019-07-18 14:48 ` [edk2-devel] " Carsey, Jaben
2019-07-19  1:09 ` Gao, Zhichao

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=20190718123142.5696-6-krzysztof.koch@arm.com \
    --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