From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=723yysYA; spf=pass (domain: arm.com, ip: 40.107.5.80, mailfrom: krzysztof.koch@arm.com) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (EUR03-VE1-obe.outbound.protection.outlook.com [40.107.5.80]) by groups.io with SMTP; Thu, 11 Jul 2019 23:55:29 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bZaol0biFPYPI/g4t9qWeFHqFb9yNnwOdjQa0hRKfJM=; b=723yysYAdplS+8TBT+v/zqNFgP+D9ANbdEWsaLWz4qhVb9CFJy4gNUjz81VRly3K2Rk2XGO5/eLi2mYmlCxKxFuzYSji8ow8RESeYS4MqyD3k+dKzu8jCdWrOg/K+msnj4po3i/d1WMBYWHQ06gUDTTK0xHe8H2+nbsED17vS/Y= Received: from VI1PR08CA0094.eurprd08.prod.outlook.com (2603:10a6:800:d3::20) by AM5PR0802MB2593.eurprd08.prod.outlook.com (2603:10a6:203:94::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2052.16; Fri, 12 Jul 2019 06:55:25 +0000 Received: from DB5EUR03FT060.eop-EUR03.prod.protection.outlook.com (2a01:111:f400:7e0a::203) by VI1PR08CA0094.outlook.office365.com (2603:10a6:800:d3::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.2073.10 via Frontend Transport; Fri, 12 Jul 2019 06:55:24 +0000 Authentication-Results: spf=temperror (sender IP is 40.67.248.234) smtp.mailfrom=arm.com; edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=temperror action=none header.from=arm.com; Received-SPF: TempError (protection.outlook.com: error in processing during lookup of arm.com: DNS Timeout) Received: from nebula.arm.com (40.67.248.234) by DB5EUR03FT060.mail.protection.outlook.com (10.152.21.231) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.20.2052.18 via Frontend Transport; Fri, 12 Jul 2019 06:55:23 +0000 Received: from AZ-NEU-EX03.Arm.com (10.251.24.31) by AZ-NEU-EX04.Arm.com (10.251.24.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1415.2; Fri, 12 Jul 2019 06:53:23 +0000 Received: from E119924.Arm.com (10.37.8.167) by mail.arm.com (10.251.24.31) with Microsoft SMTP Server id 15.1.1415.2 via Frontend Transport; Fri, 12 Jul 2019 06:53:22 +0000 From: "Krzysztof Koch" To: CC: , , , , , Subject: [PATCH v1 10/11] ShellPkg: acpiview: GTDT: Add error-checking in the parsing logic Date: Fri, 12 Jul 2019 07:52:42 +0100 Message-ID: <20190712065243.3812-11-krzysztof.koch@arm.com> X-Mailer: git-send-email 2.16.2.windows.1 In-Reply-To: <20190712065243.3812-1-krzysztof.koch@arm.com> References: <20190712065243.3812-1-krzysztof.koch@arm.com> Return-Path: Krzysztof.Koch@arm.com MIME-Version: 1.0 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-Forefront-Antispam-Report: CIP:40.67.248.234;IPV:NLI;CTRY:IE;EFV:NLI;SFV:NSPM;SFS:(10009020)(4636009)(376002)(346002)(396003)(39860400002)(136003)(2980300002)(199004)(189003)(44832011)(2906002)(36756003)(8676002)(81156014)(486006)(6306002)(356004)(81166006)(6666004)(86362001)(305945005)(186003)(2351001)(48376002)(50466002)(68736007)(63350400001)(4326008)(126002)(476003)(1076003)(53936002)(47776003)(8936002)(70586007)(70206006)(54906003)(966005)(76176011)(53416004)(478600001)(7696005)(51416003)(336012)(50226002)(446003)(2616005)(6916009)(11346002)(26005)(16586007)(426003)(316002)(5660300002)(63370400001);DIR:OUT;SFP:1101;SCL:1;SRVR:AM5PR0802MB2593;H:nebula.arm.com;FPR:;SPF:TempError;LANG:en;PTR:InfoDomainNonexistent;A:1;MX:1; X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 1471643a-c935-4bd5-cfe1-08d70695e95a X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328);SRVR:AM5PR0802MB2593; X-MS-TrafficTypeDiagnostic: AM5PR0802MB2593: X-MS-Exchange-PUrlCount: 1 X-Microsoft-Antispam-PRVS: NoDisclaimer: True X-MS-Oob-TLC-OOBClassifiers: OLM:7691; X-Forefront-PRVS: 00963989E5 X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Message-Info: +R9nIMYsw8dCl6Sh9kkQJ/4UZj3v6g0ZeGe8fY3l2K3pBgQli4g8IscVuK5C0OmUyHLiHMFWBlwN0TYtKDQUp8zIOC7O8WnT87syLABdUJbHFtyKRA3G5z5WtS79AqIQBtnvtFdYxLbQNFZ9/SVzlJnlM5/7Btub4JUIqFICcc4FeEjYBR0hs5ye0aae1MFOLV9OjB+mN9GQB+9EdQM/IyNEMQsn5+ZHsrP0RdVOLCt9qwY0uZdzUTuQMM0itFVZMMo9cmJSURGMZhXmIN96tzOKOrJrjuAkkcZcd4k0mFOu45Qn7gse+BGKk812gQcCNWtCdPV6I9l2ax02oYfeBygM76kSLHCtePl3CbKG24n3miwGSEXDIHL1m3KfVIAfqrh4KgfANAOY66iOfoW5eZzcmOFWCAIP5naDBG/e9qI= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2019 06:55:23.1020 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 1471643a-c935-4bd5-cfe1-08d70695e95a X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[40.67.248.234];Helo=[nebula.arm.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2593 Content-Type: text/plain 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. Remove redundant forward function declarations by repositioning blocks of code. 5. Convert a 'do-while' loop for parsing GTDT table body into a 'while' block for consistency with other table parsers. Signed-off-by: Krzysztof Koch --- Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/8c2ed18c7f1c44620eb86e1c9117cbccee8938ce Notes: v1: - improve the logic in the GTDT parser [Krzysztof] ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 294 +++++++++++--------- 1 file changed, 170 insertions(+), 124 deletions(-) diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c index 3b05ff3015d4a3af62dd9fab057c32369a456267..4e8e6f3eb50596823827d20dbb72314a583d0931 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c @@ -12,6 +12,10 @@ #include #include "AcpiParser.h" #include "AcpiTableParser.h" +#include "AcpiView.h" + +// "The number of GT Block Timers must be less than or equal to 8" +#define GT_BLOCK_TIMER_COUNT_MAX 8 // Local variables STATIC CONST UINT32* GtdtPlatformTimerCount; @@ -20,7 +24,6 @@ STATIC CONST UINT8* PlatformTimerType; STATIC CONST UINT16* PlatformTimerLength; STATIC CONST UINT32* GtBlockTimerCount; STATIC CONST UINT32* GtBlockTimerOffset; -STATIC CONST UINT16* GtBlockLength; STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo; /** @@ -36,7 +39,21 @@ EFIAPI ValidateGtBlockTimerCount ( IN UINT8* Ptr, IN VOID* Context - ); + ) +{ + UINT32 BlockTimerCount; + + BlockTimerCount = *(UINT32*)Ptr; + + if (BlockTimerCount > GT_BLOCK_TIMER_COUNT_MAX) { + IncrementErrorCount (); + Print ( + L"\nERROR: Timer Count = %d. Max Timer Count is %d.", + BlockTimerCount, + GT_BLOCK_TIMER_COUNT_MAX + ); + } +} /** This function validates the GT Frame Number. @@ -51,7 +68,21 @@ EFIAPI ValidateGtFrameNumber ( IN UINT8* Ptr, IN VOID* Context - ); + ) +{ + UINT8 FrameNumber; + + FrameNumber = *(UINT8*)Ptr; + + if (FrameNumber >= GT_BLOCK_TIMER_COUNT_MAX) { + IncrementErrorCount (); + Print ( + L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-%d.", + FrameNumber, + GT_BLOCK_TIMER_COUNT_MAX - 1 + ); + } +} /** An ACPI_PARSER array describing the ACPI GTDT Table. @@ -96,7 +127,7 @@ STATIC CONST ACPI_PARSER GtPlatformTimerHeaderParser[] = { **/ STATIC CONST ACPI_PARSER GtBlockParser[] = { {L"Type", 1, 0, L"%d", NULL, NULL, NULL, NULL}, - {L"Length", 2, 1, L"%d", NULL, (VOID**)&GtBlockLength, NULL, NULL}, + {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL}, {L"Reserved", 1, 3, L"%x", NULL, NULL, NULL, NULL}, {L"Physical address (CntCtlBase)", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL}, {L"Timer Count", 4, 12, L"%d", NULL, (VOID**)&GtBlockTimerCount, @@ -134,115 +165,71 @@ STATIC CONST ACPI_PARSER SBSAGenericWatchdogParser[] = { {L"Watchdog Timer Flags", 4, 24, L"0x%x", NULL, NULL, NULL, NULL} }; -/** - This function validates the GT Block timer count. - - @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 -ValidateGtBlockTimerCount ( - IN UINT8* Ptr, - IN VOID* Context - ) -{ - UINT32 BlockTimerCount; - - BlockTimerCount = *(UINT32*)Ptr; - - if (BlockTimerCount > 8) { - IncrementErrorCount (); - Print ( - L"\nERROR: Timer Count = %d. Max Timer Count is 8.", - BlockTimerCount - ); - } -} - -/** - This function validates the GT Frame Number. - - @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 -ValidateGtFrameNumber ( - IN UINT8* Ptr, - IN VOID* Context - ) -{ - UINT8 FrameNumber; - - FrameNumber = *(UINT8*)Ptr; - - if (FrameNumber > 7) { - IncrementErrorCount (); - Print ( - L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-7.", - FrameNumber - ); - } -} - /** This function parses the Platform GT Block. - @param [in] Ptr Pointer to the start of the GT Block data. - @param [in] Length Length of the GT Block structure. + @param [in] Ptr Pointer to the start of the GT Block data. + @param [in] Length Length of the GT Block structure. **/ STATIC VOID DumpGTBlock ( IN UINT8* Ptr, - IN UINT32 Length + IN UINT16 Length ) { UINT32 Index; UINT32 Offset; - UINT32 GTBlockTimerLength; - Offset = ParseAcpi ( - TRUE, - 2, - "GT Block", - Ptr, - Length, - PARSER_PARAMS (GtBlockParser) - ); - GTBlockTimerLength = (*GtBlockLength - Offset) / (*GtBlockTimerCount); - Length -= Offset; + ParseAcpi ( + TRUE, + 2, + "GT Block", + Ptr, + Length, + PARSER_PARAMS (GtBlockParser) + ); - if (*GtBlockTimerCount != 0) { - Ptr += (*GtBlockTimerOffset); - Index = 0; - while ((Index < (*GtBlockTimerCount)) && (Length >= GTBlockTimerLength)) { - Offset = ParseAcpi ( - TRUE, - 2, - "GT Block Timer", - Ptr, - GTBlockTimerLength, - PARSER_PARAMS (GtBlockTimerParser) - ); - // Increment by GT Block Timer structure size - Ptr += Offset; - Length -= Offset; - Index++; - } + // Check if the values used to control the parsing logic have been + // successfully read. + if ((GtBlockTimerCount == NULL) || + (GtBlockTimerOffset == NULL)) { + IncrementErrorCount (); + Print ( + L"ERROR: Insufficient GT Block Structure length. Length = %d.\n", + Length + ); + return; + } - if (Length != 0) { - IncrementErrorCount (); - Print ( - L"ERROR:GT Block Timer length mismatch. Unparsed %d bytes.\n", - Length - ); - } + Offset = *GtBlockTimerOffset; + Index = 0; + + // Parse the specified number of GT Block Timer Structures or the GT Block + // Structure buffer length. Whichever is minimum. + while ((Index++ < *GtBlockTimerCount) && + (Offset < Length)) { + Offset += ParseAcpi ( + TRUE, + 2, + "GT Block Timer", + Ptr + Offset, + Length - Offset, + PARSER_PARAMS (GtBlockTimerParser) + ); + } + + // Cross-check the substructure count with the length of the encapsulating + // buffer + if (GetConsistencyChecking () && + (Index < *GtBlockTimerCount)) { + IncrementErrorCount (); + Print ( + L"ERROR: Invalid GT Block Timer count. GtBlockTimerCount = %d. " \ + L"GtBlockBufferSize = %d.\n", + *GtBlockTimerCount, + Length + ); } } @@ -295,6 +282,7 @@ ParseAcpiGtdt ( ) { UINT32 Index; + UINT32 Offset; UINT8* TimerPtr; if (!Trace) { @@ -310,36 +298,94 @@ ParseAcpiGtdt ( PARSER_PARAMS (GtdtParser) ); - if (*GtdtPlatformTimerCount != 0) { - TimerPtr = Ptr + (*GtdtPlatformTimerOffset); - Index = 0; - do { - // Parse the Platform Timer Header - ParseAcpi ( - FALSE, - 0, - NULL, - TimerPtr, - 4, // GT Platform Timer structure header length. - PARSER_PARAMS (GtPlatformTimerHeaderParser) + // Check if the values used to control the parsing logic have been + // successfully read. + if ((GtdtPlatformTimerCount == NULL) || + (GtdtPlatformTimerOffset == NULL)) { + IncrementErrorCount (); + Print ( + L"ERROR: Insufficient table length. AcpiTableLength = %d.\n", + AcpiTableLength + ); + return; + } + + TimerPtr = Ptr + *GtdtPlatformTimerOffset; + Offset = *GtdtPlatformTimerOffset; + Index = 0; + + // Parse the specified number of Platform Timer Structures or the GTDT + // buffer length. Whichever is minimum. + while ((Index++ < *GtdtPlatformTimerCount) && + (Offset < AcpiTableLength)) { + // Parse the Platform Timer Header to obtain Length and Type + ParseAcpi ( + FALSE, + 0, + NULL, + TimerPtr, + AcpiTableLength - Offset, + PARSER_PARAMS (GtPlatformTimerHeaderParser) + ); + + // Check if the values used to control the parsing logic have been + // successfully read. + if ((PlatformTimerType == NULL) || + (PlatformTimerLength == NULL)) { + IncrementErrorCount (); + Print ( + L"ERROR: Insufficient remaining table buffer length to read the " \ + L"Platform Timer Structure header. Length = %d.\n", + AcpiTableLength - Offset ); - switch (*PlatformTimerType) { - case EFI_ACPI_6_2_GTDT_GT_BLOCK: - DumpGTBlock (TimerPtr, *PlatformTimerLength); - break; - case EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG: - DumpWatchdogTimer (TimerPtr, *PlatformTimerLength); - break; - default: + return; + } + + // Make sure the Platform Timer is inside the table. + if ((Offset + *PlatformTimerLength) > AcpiTableLength) { + IncrementErrorCount (); + Print ( + L"ERROR: Invalid Platform Timer Structure length. " \ + L"PlatformTimerLength = %d. RemainingTableBufferLength = %d. " \ + L"GTDT parsing aborted.\n", + *PlatformTimerLength, + AcpiTableLength - Offset + ); + return; + } + + switch (*PlatformTimerType) { + case EFI_ACPI_6_3_GTDT_GT_BLOCK: + DumpGTBlock (TimerPtr, *PlatformTimerLength); + break; + case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG: + DumpWatchdogTimer (TimerPtr, *PlatformTimerLength); + break; + default: + if (GetConsistencyChecking ()) { IncrementErrorCount (); Print ( - L"ERROR: INVALID Platform Timer Type = %d\n", + L"ERROR: Invalid Platform Timer Type = %d\n", *PlatformTimerType ); - break; - } // switch - TimerPtr += (*PlatformTimerLength); - Index++; - } while (Index < *GtdtPlatformTimerCount); + } + break; + } // switch + + TimerPtr += *PlatformTimerLength; + Offset += *PlatformTimerLength; + } // while + + // Cross-check the substructure count with the length of the encapsulating + // buffer + if (GetConsistencyChecking () && + (Index < *GtdtPlatformTimerCount)) { + IncrementErrorCount (); + Print ( + L"ERROR: Invalid Platform Timer Structure count. " \ + L"GtdtPlatformTimerCount = %d. AcpiTableLength = %d.\n", + *GtdtPlatformTimerCount, + AcpiTableLength + ); } } -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'