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 03/11] ShellPkg: acpiview: RSDP: Make printing table checksum optional
Date: Fri, 12 Jul 2019 07:52:35 +0100	[thread overview]
Message-ID: <20190712065243.3812-4-krzysztof.koch@arm.com> (raw)
In-Reply-To: <20190712065243.3812-1-krzysztof.koch@arm.com>

1. Don't validate Root System Description Pointer (RSDP) structure
checksum if the '-q' command line flag is used with the acpiview UEFI
shell tool. This change makes the RSDP parser consistent with the
parsers for other ACPI tables.

2. Check if XsdtAddress pointer has been successfully updated before it
is used for further table parsing.

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

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/73e6d7e117da244f8f4065620115a47f7f66d372

Notes:
    v1:
    - make RSDP parser behavior consistent with other tables [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c | 144 +++++++++-----------
 1 file changed, 68 insertions(+), 76 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
index 586de7cbfb12f856c0c735b6e295c1cc32eb2ceb..952517cd09aaff601bb363fd73331c750a9e97ff 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
@@ -1,7 +1,7 @@
 /** @file
   RSDP 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):
@@ -11,6 +11,7 @@
 #include <Library/UefiLib.h>
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "AcpiView.h"
 
 // Local Variables
 STATIC CONST UINT64* XsdtAddress;
@@ -28,7 +29,27 @@ EFIAPI
 ValidateRsdtAddress (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  // Reference: Server Base Boot Requirements System Software on ARM Platforms
+  // Section: 4.2.1.1 RSDP
+  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
+  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
+  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
+  UINT32 RsdtAddr;
+
+  RsdtAddr = *(UINT32*)Ptr;
+
+  if (RsdtAddr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Rsdt Address = 0x%p. This must be NULL on ARM Platforms.",
+      RsdtAddr
+      );
+  }
+#endif
+}
 
 /**
   This function validates the XSDT Address.
@@ -43,7 +64,27 @@ EFIAPI
 ValidateXsdtAddress (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  // Reference: Server Base Boot Requirements System Software on ARM Platforms
+  // Section: 4.2.1.1 RSDP
+  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
+  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
+  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
+  UINT64 XsdtAddr;
+
+  XsdtAddr = *(UINT64*)Ptr;
+
+  if (XsdtAddr == 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Xsdt Address = 0x%p. This must not be NULL on ARM Platforms.",
+      XsdtAddr
+      );
+  }
+#endif
+}
 
 /**
   An array describing the ACPI RSDP Table.
@@ -61,76 +102,6 @@ STATIC CONST ACPI_PARSER RsdpParser[] = {
   {L"Reserved", 3, 33, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the RSDT Address.
-
-  @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
-ValidateRsdtAddress (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  // Reference: Server Base Boot Requirements System Software on ARM Platforms
-  // Section: 4.2.1.1 RSDP
-  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
-  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
-  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
-  UINT32 RsdtAddr;
-
-  RsdtAddr = *(UINT32*)Ptr;
-
-  if (RsdtAddr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Rsdt Address = 0x%p. This must be NULL on ARM Platforms.",
-      RsdtAddr
-      );
-  }
-#endif
-}
-
-/**
-  This function validates the XSDT Address.
-
-  @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
-ValidateXsdtAddress (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  // Reference: Server Base Boot Requirements System Software on ARM Platforms
-  // Section: 4.2.1.1 RSDP
-  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
-  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
-  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
-  UINT64 XsdtAddr;
-
-  XsdtAddr = *(UINT64*)Ptr;
-
-  if (XsdtAddr == 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Xsdt Address = 0x%p. This must not be NULL on ARM Platforms.",
-      XsdtAddr
-      );
-  }
-#endif
-}
-
 /**
   This function parses the ACPI RSDP table.
 
@@ -156,10 +127,31 @@ ParseAcpiRsdp (
 {
   if (Trace) {
     DumpRaw (Ptr, AcpiTableLength);
-    VerifyChecksum (TRUE, Ptr, AcpiTableLength);
+    if (GetConsistencyChecking ()) {
+      VerifyChecksum (TRUE, Ptr, AcpiTableLength);
+    }
   }
 
-  ParseAcpi (Trace, 0, "RSDP", Ptr, AcpiTableLength, PARSER_PARAMS (RsdpParser));
+  ParseAcpi (
+    Trace,
+    0,
+    "RSDP",
+    Ptr,
+    AcpiTableLength,
+    PARSER_PARAMS (RsdpParser)
+    );
+
+  // Check if the values used to control the parsing logic have been
+  // successfully read.
+  if (XsdtAddress == NULL) {
+    IncrementErrorCount ();
+    Print (
+      L"ERROR: Insufficient table length. AcpiTableLength = %d." \
+        L"RSDP parsing aborted.\n",
+      AcpiTableLength
+      );
+    return;
+  }
 
   // This code currently supports parsing of XSDT table only
   // and does not parse the RSDT table. Platforms provide the
@@ -167,7 +159,7 @@ ParseAcpiRsdp (
   // Therefore the RSDT should not be used on ARM platforms.
   if ((*XsdtAddress) == 0) {
     IncrementErrorCount ();
-    Print (L"ERROR: XSDT Pointer is not set.\n");
+    Print (L"ERROR: XSDT Pointer is not set. RSDP parsing aborted.\n");
     return;
   }
 
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



  parent 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 ` [PATCH v1 01/11] ShellPkg: acpiview: FADT: Validate global pointers before use Krzysztof Koch
2019-07-12 14:26   ` 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 ` Krzysztof Koch [this message]
2019-07-17  9:41   ` [edk2-devel] [PATCH v1 03/11] ShellPkg: acpiview: RSDP: Make printing table checksum optional 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-4-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