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 01/11] ShellPkg: acpiview: FADT: Validate global pointers before use
Date: Fri, 12 Jul 2019 07:52:33 +0100	[thread overview]
Message-ID: <20190712065243.3812-2-krzysztof.koch@arm.com> (raw)
In-Reply-To: <20190712065243.3812-1-krzysztof.koch@arm.com>

1. Check if the global pointer have been successfully updated before
they are later used to control the parsing logic in the FADT acpiview
parser.

2. Remove redundant forward function declarations by repositioning
blocks of code.

3. Allow silencing ACPI table content validation errors which do not
cause table parsing to fail.

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/49cc41430775fb93205e302590a7d31f080c3952

Notes:
    v1:
    - improve the logic in the parser [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 131 ++++++++------------
 1 file changed, 51 insertions(+), 80 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
index cee7ee0770433da96d6042d2f5d687903f4b5495..600d3b16d7b22b61c1a1fd21ecb93f16c7f8fa1a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
@@ -1,7 +1,7 @@
 /** @file
   FADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -12,6 +12,7 @@
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "AcpiView.h"
 
 // Local variables
 STATIC CONST UINT32* DsdtAddress;
@@ -46,7 +47,17 @@ EFIAPI
 ValidateFirmwareCtrl (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  if (*(UINT32*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Firmware Control must be zero for ARM platforms."
+    );
+  }
+#endif
+}
 
 /**
   This function validates the X_Firmware Control Field.
@@ -61,7 +72,17 @@ EFIAPI
 ValidateXFirmwareCtrl (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  if (*(UINT64*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: X Firmware Control must be zero for ARM platforms."
+    );
+  }
+#endif
+}
 
 /**
   This function validates the flags.
@@ -76,7 +97,17 @@ EFIAPI
 ValidateFlags (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) == 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms."
+    );
+  }
+#endif
+}
 
 /**
   An ACPI_PARSER array describing the ACPI FADT Table.
@@ -142,81 +173,6 @@ STATIC CONST ACPI_PARSER FadtParser[] = {
   {L"Hypervisor VendorIdentity", 8, 268, L"%lx", NULL, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the Firmware Control Field.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateFirmwareCtrl (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Firmware Control must be zero for ARM platforms."
-    );
-  }
-#endif
-}
-
-/**
-  This function validates the X_Firmware Control Field.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateXFirmwareCtrl (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (*(UINT64*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: X Firmware Control must be zero for ARM platforms."
-    );
-  }
-#endif
-}
-
-/**
-  This function validates the flags.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateFlags (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) == 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms."
-    );
-  }
-#endif
-}
-
 /**
   This function parses the ACPI FADT table.
   This function parses the FADT table and optionally traces the ACPI table fields.
@@ -248,12 +204,27 @@ ParseAcpiFadt (
     PARSER_PARAMS (FadtParser)
     );
 
+  // Check if the values used to control the parsing logic have been
+  // successfully read.
+  if ((DsdtAddress == NULL)       ||
+      (FadtMinorRevision == NULL) ||
+      (X_DsdtAddress == NULL)) {
+    IncrementErrorCount ();
+    Print (
+      L"ERROR: Insufficient table length. AcpiTableLength = %d. " \
+        L"FADT parsing aborted.\n",
+      AcpiTableLength
+      );
+    return;
+  }
+
   if (Trace) {
     Print (L"\nSummary:\n");
     PrintFieldName (2, L"FADT Version");
     Print (L"%d.%d\n",  *AcpiHdrInfo.Revision, *FadtMinorRevision);
 
-    if (*GetAcpiXsdtHeaderInfo ()->OemTableId != *AcpiHdrInfo.OemTableId) {
+    if (GetConsistencyChecking () &&
+        (*GetAcpiXsdtHeaderInfo ()->OemTableId != *AcpiHdrInfo.OemTableId)) {
       IncrementErrorCount ();
       Print (L"ERROR: OEM Table Id does not match with RSDT/XSDT.\n");
     }
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



  reply	other threads:[~2019-07-12  6:53 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12  6:52 [PATCH v1 00/11] Add security checks in the Acpiview table parsers Krzysztof Koch
2019-07-12  6:52 ` Krzysztof Koch [this message]
2019-07-12 14:26   ` [PATCH v1 01/11] ShellPkg: acpiview: FADT: Validate global pointers before use Carsey, Jaben
2019-07-17 13:38     ` [edk2-devel] " Krzysztof Koch
2019-07-19  1:21   ` Gao, Zhichao
2019-07-12  6:52 ` [PATCH v1 02/11] ShellPkg: acpiview: SPCR: Remove redundant forward declaration Krzysztof Koch
2019-07-17  9:42   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 03/11] ShellPkg: acpiview: RSDP: Make printing table checksum optional Krzysztof Koch
2019-07-17  9:41   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 04/11] ShellPkg: acpiview: XSDT: Remove redundant ParseAcpi() call Krzysztof Koch
2019-07-17  9:41   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 05/11] ShellPkg: acpiview: SLIT: Add error-checking in the parsing logic Krzysztof Koch
2019-07-17  9:42   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 06/11] ShellPkg: acpiview: SRAT: " Krzysztof Koch
2019-07-17  9:41   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 07/11] ShellPkg: acpiview: MADT: " Krzysztof Koch
2019-07-17  9:40   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 08/11] ShellPkg: acpiview: PPTT: " Krzysztof Koch
2019-07-17  9:40   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 09/11] ShellPkg: acpiview: IORT: " Krzysztof Koch
2019-07-17  9:40   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 10/11] ShellPkg: acpiview: GTDT: " Krzysztof Koch
2019-07-17  9:39   ` [edk2-devel] " Alexei Fedorov
2019-07-12  6:52 ` [PATCH v1 11/11] ShellPkg: acpiview: DBG2: " Krzysztof Koch
2019-07-17  9:39   ` [edk2-devel] " Alexei Fedorov
2019-07-19  5:39   ` Gao, Zhichao
2019-07-17  9:42 ` [edk2-devel] [PATCH v1 00/11] Add security checks in the Acpiview table parsers Alexei Fedorov

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=20190712065243.3812-2-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