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: PPTT: Prevent buffer overruns
Date: Thu, 1 Aug 2019 09:44:06 +0100	[thread overview]
Message-ID: <20190801084407.48712-6-krzysztof.koch@arm.com> (raw)
In-Reply-To: <20190801084407.48712-1-krzysztof.koch@arm.com>

Modify the PPTT table parsing logic to prevent reading past the ACPI
buffer lengths provided.

Check if the Number of Private Resources specified in the Processor
Hierarchy Node (Type 0) is possible given the Type 0 Structure's buffer
length.

Make sure that the processor topology structure's buffer fits in the
PPTT table buffer before its contents are dumped.

Prevent buffer overruns when reading the processor topology structure's
header.

References:
- ACPI 6.3, January 2019, Section 5.2.29

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

Notes:
    v1:
    - Prevent buffer overruns in PPTT acpiview parser [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 38 ++++++++++++++------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index cec57be55e77096f9448f637ea129af2b42111ad..6254b9913fffb429fc54bb1301bf3e4b2e5bf161 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -252,7 +252,6 @@ DumpProcessorHierarchyNodeStructure (
   )
 {
   UINT32 Offset;
-  UINT8* PrivateResourcePtr;
   UINT32 Index;
   CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
 
@@ -265,8 +264,23 @@ DumpProcessorHierarchyNodeStructure (
              PARSER_PARAMS (ProcessorHierarchyNodeStructureParser)
              );
 
-  PrivateResourcePtr = Ptr + Offset;
+  // Make sure the Private Resource array lies inside this structure
+  if (Offset + (*NumberOfPrivateResources * sizeof (UINT32)) > Length) {
+    IncrementErrorCount ();
+    Print (
+      L"ERROR: Invalid Number of Private Resources. " \
+        L"PrivateResourceCount = %d. RemainingBufferLength = %d. " \
+        L"Parsing of this structure aborted.\n",
+      *NumberOfPrivateResources,
+      Length - Offset
+      );
+    return;
+  }
+
   Index = 0;
+
+  // Parse the specified number of private resource references or the Processor
+  // Hierarchy Node length. Whichever is minimum.
   while (Index < *NumberOfPrivateResources) {
     UnicodeSPrint (
       Buffer,
@@ -278,10 +292,10 @@ DumpProcessorHierarchyNodeStructure (
     PrintFieldName (4, Buffer);
     Print (
       L"0x%x\n",
-      *((UINT32*) PrivateResourcePtr)
+      *((UINT32*)(Ptr + Offset))
       );
 
-    PrivateResourcePtr += sizeof(UINT32);
+    Offset += sizeof (UINT32);
     Index++;
   }
 }
@@ -382,19 +396,21 @@ ParseAcpiPptt (
       0,
       NULL,
       ProcessorTopologyStructurePtr,
-      4,  // Length of the processor topology structure header is 4 bytes
+      AcpiTableLength - Offset,
       PARSER_PARAMS (ProcessorTopologyStructureHeaderParser)
       );
 
-    if ((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength) {
+    // Make sure the PPTT structure lies inside the table
+    if ((Offset + *ProcessorTopologyStructureLength) > AcpiTableLength) {
       IncrementErrorCount ();
       Print (
-        L"ERROR: Invalid processor topology structure length:"
-          L" Type = %d, Length = %d\n",
-        *ProcessorTopologyStructureType,
-        *ProcessorTopologyStructureLength
+        L"ERROR: Invalid PPTT structure length. " \
+          L"ProcessorTopologyStructureLength = %d. " \
+          L"RemainingTableBufferLength = %d. PPTT parsing aborted.\n",
+        *ProcessorTopologyStructureLength,
+        AcpiTableLength - Offset
         );
-      break;
+      return;
     }
 
     PrintFieldName (2, L"* Structure Offset *");
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



  parent reply	other threads:[~2019-08-01  8:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-01  8:44 [PATCH v1 0/6] Prevent buffer overruns in acpiview table parsers Krzysztof Koch
2019-08-01  8:44 ` [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns Krzysztof Koch
2019-08-01  9:33   ` [edk2-devel] " Alexei Fedorov
2019-08-05  6:48   ` Gao, Zhichao
2019-08-05  8:21     ` Krzysztof Koch
2019-08-06  7:43       ` [edk2-devel] " Gao, Zhichao
2019-08-06 10:44         ` Krzysztof Koch
2019-08-07  1:52           ` Gao, Zhichao
2019-08-01  8:44 ` [PATCH v1 2/6] ShellPkg: acpiview: GTDT: " Krzysztof Koch
2019-08-01  9:33   ` [edk2-devel] " Alexei Fedorov
2019-08-05  7:23   ` Gao, Zhichao
2019-08-05  9:54     ` Sami Mujawar
2019-08-06  4:58       ` Gao, Zhichao
2019-08-01  8:44 ` [PATCH v1 3/6] ShellPkg: acpiview: IORT: " Krzysztof Koch
2019-08-01  9:32   ` [edk2-devel] " Alexei Fedorov
2019-08-01  8:44 ` [PATCH v1 4/6] ShellPkg: acpiview: MADT: " Krzysztof Koch
2019-08-01  9:32   ` [edk2-devel] " Alexei Fedorov
2019-08-01  8:44 ` Krzysztof Koch [this message]
2019-08-01  9:31   ` [edk2-devel] [PATCH v1 5/6] ShellPkg: acpiview: PPTT: " Alexei Fedorov
2019-08-01  8:44 ` [PATCH v1 6/6] ShellPkg: acpiview: SRAT: " Krzysztof Koch
2019-08-01  9:30   ` [edk2-devel] " Alexei Fedorov
2019-08-01  9:33 ` [edk2-devel] [PATCH v1 0/6] Prevent buffer overruns in acpiview table parsers Alexei Fedorov
2019-08-06  8:10 ` Gao, Zhichao
2019-08-07  8:46 ` Sami Mujawar

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=20190801084407.48712-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