[edk2-devel] [PATCH v1 05/11] ShellPkg: acpiview: SLIT: Add error-checking in the parsing logic
Krzysztof Koch
krzysztof.koch at arm.com
Fri Jul 12 06:52:37 UTC 2019
1. Check if the global pointers (in the scope of this ACPI table parser)
have been successfully updated before they are later used to control
the parsing logic.
2. Test against buffer overruns.
3. Allow silencing ACPI table content validation errors which do not
cause table parsing to fail.
4. Check if the 'Number of System Localities' provided can be
represented in a SLIT table. The table 'Length' field is a 32-bit value
while the 'Number of System Localities' is a 64-bit value.
Signed-off-by: Krzysztof Koch <krzysztof.koch at arm.com>
---
Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/2f1ffd1a74d06c23c01971be965d856f0a0e3ac4
Notes:
v1:
- improve the logic in the SLIT parser [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c | 115 ++++++++++++++------
1 file changed, 83 insertions(+), 32 deletions(-)
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index 1f9dac66eea5b0f4366a7a9584ac6702a74beaac..dd5f039b67326acc710ee703a6b132a1e280dcaa 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
@@ -1,7 +1,7 @@
/** @file
SLIT 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):
@@ -13,6 +13,7 @@
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
// Local Variables
STATIC CONST UINT64* SlitSystemLocalityCount;
@@ -59,7 +60,7 @@ ParseAcpiSlit (
UINT32 Offset;
UINT64 Count;
UINT64 Index;
- UINT64 LocalityCount;
+ UINT32 LocalityCount;
UINT8* LocalityPtr;
CHAR16 Buffer[80]; // Used for AsciiName param of ParseAcpi
@@ -77,7 +78,55 @@ ParseAcpiSlit (
);
LocalityPtr = Ptr + Offset;
- LocalityCount = *SlitSystemLocalityCount;
+ // Check if the values used to control the parsing logic have been
+ // successfully read.
+ if (SlitSystemLocalityCount == NULL) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Insufficient table length. AcpiTableLength = %d.\n",
+ AcpiTableLength
+ );
+ return;
+ }
+
+ /*
+ Despite the 'Number of System Localities' being a 64-bit field in SLIT,
+ the maximum number of localities that can be represented in SLIT is limited
+ by the 'Length' field of the ACPI table.
+
+ Since the ACPI table length field is 32-bit wide. The maximum number of
+ localities that can be represented in SLIT can be calculated as:
+
+ MaxLocality = sqrt (MAX_UINT32 - sizeof (EFI_ACPI_6_3_SYSTEM_LOCALITY_DISTANCE_INFORMATION_TABLE_HEADER))
+ = 65535
+ = MAX_UINT16
+ */
+ if (*SlitSystemLocalityCount > MAX_UINT16) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: The Number of System Localities provided can't be represented " \
+ L"in the SLIT table. SlitSystemLocalityCount = %ld. " \
+ L"MaxLocalityCountAllowed = %d.\n",
+ *SlitSystemLocalityCount,
+ MAX_UINT16
+ );
+ return;
+ }
+
+ LocalityCount = (UINT32)*SlitSystemLocalityCount;
+
+ // Make sure system localities fit in the table buffer provided
+ if (Offset + (LocalityCount * LocalityCount) > AcpiTableLength) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Invalid Number of System Localities. " \
+ L"SlitSystemLocalityCount = %ld. AcpiTableLength = %d.\n",
+ *SlitSystemLocalityCount,
+ AcpiTableLength
+ );
+ return;
+ }
+
// We only print the Localities if the count is less than 16
// If the locality count is more than 16 then refer to the
// raw data dump.
@@ -96,7 +145,7 @@ ParseAcpiSlit (
Print (L" (%3d) ", Index);
}
Print (L"\n");
- for (Count = 0; Count< LocalityCount; Count++) {
+ for (Count = 0; Count < LocalityCount; Count++) {
Print (L" (%3d) ", Count);
for (Index = 0; Index < LocalityCount; Index++) {
Print (L" %3d ", SLIT_ELEMENT (LocalityPtr, Count, Index));
@@ -106,34 +155,36 @@ ParseAcpiSlit (
}
// Validate
- for (Count = 0; Count < LocalityCount; Count++) {
- for (Index = 0; Index < LocalityCount; Index++) {
- // Element[x][x] must be equal to 10
- if ((Count == Index) && (SLIT_ELEMENT (LocalityPtr, Count,Index) != 10)) {
- IncrementErrorCount ();
- Print (
- L"ERROR: Diagonal Element[0x%lx][0x%lx] (%3d)."
- L" Normalized Value is not 10\n",
- Count,
- Index,
- SLIT_ELEMENT (LocalityPtr, Count, Index)
- );
- }
- // Element[i][j] must be equal to Element[j][i]
- if (SLIT_ELEMENT (LocalityPtr, Count, Index) !=
- SLIT_ELEMENT (LocalityPtr, Index, Count)) {
- IncrementErrorCount ();
- Print (
- L"ERROR: Relative distances for Element[0x%lx][0x%lx] (%3d) and \n"
- L"Element[0x%lx][0x%lx] (%3d) do not match.\n",
- Count,
- Index,
- SLIT_ELEMENT (LocalityPtr, Count, Index),
- Index,
- Count,
- SLIT_ELEMENT (LocalityPtr, Index, Count)
- );
+ if (GetConsistencyChecking ()) {
+ for (Count = 0; Count < LocalityCount; Count++) {
+ for (Index = 0; Index < LocalityCount; Index++) {
+ // Element[x][x] must be equal to 10
+ if ((Count == Index) && (SLIT_ELEMENT (LocalityPtr, Count,Index) != 10)) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Diagonal Element[0x%lx][0x%lx] (%3d)."
+ L" Normalized Value is not 10\n",
+ Count,
+ Index,
+ SLIT_ELEMENT (LocalityPtr, Count, Index)
+ );
+ }
+ // Element[i][j] must be equal to Element[j][i]
+ if (SLIT_ELEMENT (LocalityPtr, Count, Index) !=
+ SLIT_ELEMENT (LocalityPtr, Index, Count)) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Relative distances for Element[0x%lx][0x%lx] (%3d) and \n"
+ L"Element[0x%lx][0x%lx] (%3d) do not match.\n",
+ Count,
+ Index,
+ SLIT_ELEMENT (LocalityPtr, Count, Index),
+ Index,
+ Count,
+ SLIT_ELEMENT (LocalityPtr, Index, Count)
+ );
+ }
}
}
- }
+ } // if (GetConsistencyChecking ())
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43641): https://edk2.groups.io/g/devel/message/43641
Mute This Topic: https://groups.io/mt/32439502/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list