]> git.proxmox.com Git - mirror_edk2.git/commitdiff
ShellPkg: acpiview: PPTT: Prevent buffer overruns
authorKrzysztof Koch <krzysztof.koch@arm.com>
Thu, 1 Aug 2019 23:44:06 +0000 (16:44 -0700)
committerJaben Carsey <jaben.carsey@intel.com>
Mon, 12 Aug 2019 17:14:03 +0000 (10:14 -0700)
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>
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Reviewed-by: Zhichao Gao <zhichao.gao@inte.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c

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