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=tE0/nRss; spf=pass (domain: arm.com, ip: 40.107.7.40, mailfrom: krzysztof.koch@arm.com) Received: from EUR04-HE1-obe.outbound.protection.outlook.com (EUR04-HE1-obe.outbound.protection.outlook.com [40.107.7.40]) by groups.io with SMTP; Thu, 11 Jul 2019 23:54:16 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=V0FhfulshAfEVFGDXoW1KIS//QnXBn863ENllt4cQ2Bu2xzOs5/srNCWbxCWg8zTGY5JsZ4olK33/A+Hn5VxyUwN0YTZ8vuxiPY7S85Pm/WK8Fp8/VwT+c6CDlsE5XG02SmmlGP7xnFWwEftFKVRq47jUx77gPOE1Lzu1s1wmRIPk3Hq9Q18VxE63XWPPZx8x9bvTKrvlh56YESWq6UpGA588zxli9BfhpSMuehAjDp4Kx8v2Xlk2/tbkrBOsQ/wsaiIsCJcpcizQ/ArakJ7GmfPiAhS4vgMmtrcTjgxF9LI4eenYPN/5YZ8vaVyqixPCkNKeDq46dstl5zPvFWP9w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=U/XIvfXg2CULTzoWAZxzZgTzpH/jLmP55LpF9KEsAGs=; b=W4jv/Khty0+ORHJOaEshQJY1icbN92bNb06c5zuvBUipnFl/bX/KZzS6SIYnYGX/pP/zJHvj5cOfU2+9H+ZDVcDfj7FUS1/fBEquUbcG2ZSe6+7GhXUH4T8hvl70HNmJKCFf38DNHayn3QQg/jvppE9y8IABK3XhGbDSZKZ6AloeogSPbNDTjixZqILcY7hBEsaKnxF/YkeP1kQ1CuTSZEmKZ1EQKAWcZyb9yeJYBnz9W3DxROADsu6RjjJLDHYK9rkK26x9j6xNFutGH9l7VljIa2N6MSgszNQgFJY2d1fP4T/i8EzWUssqZgqQ3JCDqbufpUmxnQBHBYeNd2tA6A== ARC-Authentication-Results: i=1; mx.microsoft.com 1;spf=temperror (sender ip is 40.67.248.234) smtp.rcpttodomain=edk2.groups.io smtp.mailfrom=arm.com;dmarc=temperror action=none header.from=arm.com;dkim=none (message not signed);arc=none 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=U/XIvfXg2CULTzoWAZxzZgTzpH/jLmP55LpF9KEsAGs=; b=tE0/nRssJafQFPJApirNK6b90yp6IpdzxkmtyTOV9PjGgmKWvoDEKO4MkQRyDky5kaFvOizzTCuwbGZlRQhqXRUYJnWrDQA93xvyA5Wu0+vKgx9Nc9XsErGqRZpSRUbVFc7RQm9Dm0JvW5is88gW/imW1swCwCJhVKMIDpK1+2s= Received: from VI1PR08CA0176.eurprd08.prod.outlook.com (2603:10a6:800:d1::30) by HE1PR0802MB2601.eurprd08.prod.outlook.com (2603:10a6:3:d8::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2073.14; Fri, 12 Jul 2019 06:54:12 +0000 Received: from DB5EUR03FT010.eop-EUR03.prod.protection.outlook.com (2a01:111:f400:7e0a::209) by VI1PR08CA0176.outlook.office365.com (2603:10a6:800:d1::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.2073.11 via Frontend Transport; Fri, 12 Jul 2019 06:54:12 +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 DB5EUR03FT010.mail.protection.outlook.com (10.152.20.96) 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:54:11 +0000 Received: from AZ-NEU-EX01.Emea.Arm.com (10.251.26.4) by AZ-NEU-EX04.Arm.com (10.251.24.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1415.2; Fri, 12 Jul 2019 06:53:21 +0000 Received: from AZ-NEU-EX03.Arm.com (10.251.24.31) by AZ-NEU-EX01.Emea.Arm.com (10.251.26.4) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1415.2; Fri, 12 Jul 2019 06:53:20 +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:20 +0000 From: "Krzysztof Koch" To: CC: , , , , , Subject: [PATCH v1 07/11] ShellPkg: acpiview: MADT: Add error-checking in the parsing logic Date: Fri, 12 Jul 2019 07:52:39 +0100 Message-ID: <20190712065243.3812-8-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)(39860400002)(136003)(376002)(396003)(346002)(2980300002)(189003)(199004)(53416004)(2906002)(50226002)(126002)(36756003)(7696005)(51416003)(356004)(6666004)(81166006)(81156014)(68736007)(1076003)(478600001)(6916009)(8936002)(4326008)(53936002)(316002)(305945005)(54906003)(966005)(48376002)(26005)(5660300002)(6306002)(86362001)(16586007)(336012)(47776003)(70586007)(70206006)(44832011)(76176011)(8676002)(2351001)(186003)(476003)(2616005)(63350400001)(50466002)(486006)(446003)(426003)(63370400001)(11346002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0802MB2601;H:nebula.arm.com;FPR:;SPF:TempError;LANG:en;PTR:InfoDomainNonexistent;MX:1;A:1; X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 0b731d10-ad98-442a-dfbd-08d70695bec4 X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328);SRVR:HE1PR0802MB2601; X-MS-TrafficTypeDiagnostic: HE1PR0802MB2601: X-MS-Exchange-PUrlCount: 1 X-Microsoft-Antispam-PRVS: NoDisclaimer: True X-MS-Oob-TLC-OOBClassifiers: OLM:4125; X-Forefront-PRVS: 00963989E5 X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Message-Info: rfIJh/fAZx3QplNOzFmr9qIS/GVVoIqqcxJIUGWk/EjqIPLx5nsKiYCf7tc/ncHzfFjTjH+At7KUxnYznawBqON3t7u1SfGCdSgdpO1bOy8SRQ7bVlCZphqkdGD+lf/cVQW8UUEl0w0IP9TjboyiHe81zM2PpXpmZKvFUQlnR74Hy67RqyE4nMWb0pW66/pg0PuYWNPnG2Uk6tQKVFvBF3GbF/aEwb+ZA9ShxoAFNsTge6G/czt6y9SQDIwCc/3mwsCz4PAHk70QdC/HDuWEA7HDOFsSxqYpp5s7IhOo9f5TUFizmlNjc4AoWmUnwBU55Rlngq/jeHdAEvegqrt0VQeXRQFpsAj8bp93myeoX2nDZRlyp2Q8+NUOgv8TazEtgFeSO7y9Y1MJqB0Dy1fknMBEE9fm5/E16HMybyz5taw= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2019 06:54:11.6551 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 0b731d10-ad98-442a-dfbd-08d70695bec4 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: HE1PR0802MB2601 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. Give forward progress guarantee when parsing the MADT table. Report an error if a MADT structure is too small to be valid. Without this check, there is a possibility for the parser to enter an ifninite loop. 3. Test against buffer overruns. 4. Remove redundant forward function declarations by repositioning blocks of code. 5. Allow silencing ACPI table content validation errors which do not cause table parsing to fail. Signed-off-by: Krzysztof Koch --- Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/ef11738efc94a9c3d7270d376a2cb273bbadbba2 Notes: v1: - improve the logic in the MADT parser [Krzysztof] ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 187 ++++++++++---------- 1 file changed, 94 insertions(+), 93 deletions(-) diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c index 59c3df0cc8a080497b517baf36fc63f1e4ab866f..54f9fddc5426de5383b747ec7afd21396bcccfc9 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c @@ -15,6 +15,7 @@ #include #include "AcpiParser.h" #include "AcpiTableParser.h" +#include "AcpiView.h" #include "MadtParser.h" // Local Variables @@ -35,7 +36,15 @@ EFIAPI ValidateGICDSystemVectorBase ( IN UINT8* Ptr, IN VOID* Context - ); +) +{ + if (*(UINT32*)Ptr != 0) { + IncrementErrorCount (); + Print ( + L"\nERROR: System Vector Base must be zero." + ); + } +} /** This function validates the SPE Overflow Interrupt in the GICC. @@ -50,7 +59,41 @@ EFIAPI ValidateSpeOverflowInterrupt ( IN UINT8* Ptr, IN VOID* Context - ); + ) +{ + UINT16 SpeOverflowInterrupt; + + SpeOverflowInterrupt = *(UINT16*)Ptr; + + // SPE not supported by this processor + if (SpeOverflowInterrupt == 0) { + return; + } + + if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) || + ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) && + (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) || + (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) { + IncrementErrorCount (); + Print ( + L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID " + L"ranges of %d-%d or %d-%d (for GICv3.1 or later).", + SpeOverflowInterrupt, + ARM_PPI_ID_MIN, + ARM_PPI_ID_MAX, + ARM_PPI_ID_EXTENDED_MIN, + ARM_PPI_ID_EXTENDED_MAX + ); + } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) { + IncrementWarningCount(); + Print ( + L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA " + L"Level 3 PPI ID assignment: %d.", + SpeOverflowInterrupt, + ARM_PPI_ID_PMBIRQ + ); + } +} /** An ACPI_PARSER array describing the GICC Interrupt Controller Structure. @@ -158,78 +201,6 @@ STATIC CONST ACPI_PARSER MadtInterruptControllerHeaderParser[] = { {L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL} }; -/** - This function validates the System Vector Base in the GICD. - - @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 -ValidateGICDSystemVectorBase ( - IN UINT8* Ptr, - IN VOID* Context -) -{ - if (*(UINT32*)Ptr != 0) { - IncrementErrorCount (); - Print ( - L"\nERROR: System Vector Base must be zero." - ); - } -} - -/** - This function validates the SPE Overflow Interrupt in the GICC. - - @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 -ValidateSpeOverflowInterrupt ( - IN UINT8* Ptr, - IN VOID* Context - ) -{ - UINT16 SpeOverflowInterrupt; - - SpeOverflowInterrupt = *(UINT16*)Ptr; - - // SPE not supported by this processor - if (SpeOverflowInterrupt == 0) { - return; - } - - if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) || - ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) && - (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) || - (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) { - IncrementErrorCount (); - Print ( - L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID " - L"ranges of %d-%d or %d-%d (for GICv3.1 or later).", - SpeOverflowInterrupt, - ARM_PPI_ID_MIN, - ARM_PPI_ID_MAX, - ARM_PPI_ID_EXTENDED_MIN, - ARM_PPI_ID_EXTENDED_MAX - ); - } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) { - IncrementWarningCount(); - Print ( - L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA " - L"Level 3 PPI ID assignment: %d.", - SpeOverflowInterrupt, - ARM_PPI_ID_PMBIRQ - ); - } -} - /** This function parses the ACPI MADT table. When trace is enabled this function parses the MADT table and @@ -286,20 +257,47 @@ ParseAcpiMadt ( 0, NULL, InterruptContollerPtr, - 2, // Length is 1 byte at offset 1 + AcpiTableLength - Offset, PARSER_PARAMS (MadtInterruptControllerHeaderParser) ); - if (((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength) || - (*MadtInterruptControllerLength < 4)) { + // Check if the values used to control the parsing logic have been + // successfully read. + if ((MadtInterruptControllerType == NULL) || + (MadtInterruptControllerLength == NULL)) { IncrementErrorCount (); Print ( - L"ERROR: Invalid Interrupt Controller Length," - L" Type = %d, Length = %d\n", - *MadtInterruptControllerType, - *MadtInterruptControllerLength - ); - break; + L"ERROR: Insufficient remaining table buffer length to read the " \ + L"Interrupt Controller Structure header. Length = %d.\n", + AcpiTableLength - Offset + ); + return; + } + + // Make sure forward progress is made. + if (*MadtInterruptControllerLength < 2) { + IncrementErrorCount (); + Print ( + L"ERROR: Structure length is too small: " \ + L"MadtInterruptControllerLength = %d. " \ + L"MadtInterruptControllerType = %d. MADT parsing aborted.\n", + *MadtInterruptControllerLength, + *MadtInterruptControllerType + ); + return; + } + + // Make sure the MADT structure lies inside the table + if ((Offset + *MadtInterruptControllerLength) > AcpiTableLength) { + IncrementErrorCount (); + Print ( + L"ERROR: Invalid MADT structure length. " \ + L"MadtInterruptControllerLength = %d. " \ + L"RemainingTableBufferLength = %d. MADT parsing aborted.\n", + *MadtInterruptControllerLength, + AcpiTableLength - Offset + ); + return; } switch (*MadtInterruptControllerType) { @@ -316,11 +314,11 @@ ParseAcpiMadt ( } case EFI_ACPI_6_3_GICD: { - if (++GICDCount > 1) { + if (GetConsistencyChecking() && + (++GICDCount > 1)) { IncrementErrorCount (); Print ( - L"ERROR: Only one GICD must be present," - L" GICDCount = %d\n", + L"ERROR: Only one GICD must be present. GICDCount = %d\n", GICDCount ); } @@ -372,13 +370,16 @@ ParseAcpiMadt ( } default: { - IncrementErrorCount (); - Print ( - L"ERROR: Unknown Interrupt Controller Structure," - L" Type = %d, Length = %d\n", - *MadtInterruptControllerType, - *MadtInterruptControllerLength - ); + if (GetConsistencyChecking ()) { + IncrementErrorCount (); + Print ( + L"ERROR: Unknown Interrupt Controller Structure," + L" Type = %d, Length = %d\n", + *MadtInterruptControllerType, + *MadtInterruptControllerLength + ); + } + break; } } // switch -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'