From nobody Fri Apr 19 23:28:00 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+43647+1787277+3901457@groups.io; arc=fail (BodyHash is different from the expected one) Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1562914457509883.6555893213967; Thu, 11 Jul 2019 23:54:17 -0700 (PDT) Return-Path: X-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 X-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 X-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 Received-SPF: pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+43647+1787277+3901457@groups.io; helo=web01.groups.io; Received-SPF: TempError (protection.outlook.com: error in processing during lookup of arm.com: DNS Timeout) X-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 X-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 X-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 X-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: [edk2-devel] [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> In-Reply-To: <20190712065243.3812-1-krzysztof.koch@arm.com> References: <20190712065243.3812-1-krzysztof.koch@arm.com> MIME-Version: 1.0 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 0b731d10-ad98-442a-dfbd-08d70695bec4 X-MS-TrafficTypeDiagnostic: HE1PR0802MB2601: X-MS-Exchange-PUrlCount: 1 X-Microsoft-Antispam-PRVS: NoDisclaimer: True X-MS-Oob-TLC-OOBClassifiers: OLM:4125; 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 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,krzysztof.koch@arm.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1562914457; bh=TLUGDqeqwOMNNUHE4zyESNQpngnCDXfX3TD4PrIn9nM=; h=CC:Content-Type:Date:From:Reply-To:Subject:To; b=SBSnOJV9kXXp9egEIal17ahAqF/Pk6DNwTem7zV8AQzeFkSckW6Lni68ozY1aqAES6f 9A9hf7IT0y8VeiUl+bi62+wXjPH0+JFLE4QyNlcZcnOgOedzC/eG4Qg3PFDKmotZ/KH3p BaQYf35uvR1ltUlt+DVYpx5UmWHgF8b/DJo= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 Reviewed-by: Alexei Fedorov --- Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/ef117= 38efc94a9c3d7270d376a2cb273bbadbba2 Notes: v1: - improve the logic in the MADT parser [Krzysztof] ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 1= 87 ++++++++++---------- 1 file changed, 94 insertions(+), 93 deletions(-) diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/Madt= Parser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPa= rser.c index 59c3df0cc8a080497b517baf36fc63f1e4ab866f..54f9fddc5426de5383b747ec7af= d21396bcccfc9 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" =20 // Local Variables @@ -35,7 +36,15 @@ EFIAPI ValidateGICDSystemVectorBase ( IN UINT8* Ptr, IN VOID* Context - ); +) +{ + if (*(UINT32*)Ptr !=3D 0) { + IncrementErrorCount (); + Print ( + L"\nERROR: System Vector Base must be zero." + ); + } +} =20 /** 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 =3D *(UINT16*)Ptr; + + // SPE not supported by this processor + if (SpeOverflowInterrupt =3D=3D 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 !=3D ARM_PPI_ID_PMBIRQ) { + IncrementWarningCount(); + Print ( + L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with S= BSA " + L"Level 3 PPI ID assignment: %d.", + SpeOverflowInterrupt, + ARM_PPI_ID_PMBIRQ + ); + } +} =20 /** An ACPI_PARSER array describing the GICC Interrupt Controller Structure. @@ -158,78 +201,6 @@ STATIC CONST ACPI_PARSER MadtInterruptControllerHeader= Parser[] =3D { {L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL} }; =20 -/** - 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 !=3D 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 =3D *(UINT16*)Ptr; - - // SPE not supported by this processor - if (SpeOverflowInterrupt =3D=3D 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 !=3D ARM_PPI_ID_PMBIRQ) { - IncrementWarningCount(); - Print ( - L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with S= BSA " - 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) ); =20 - if (((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength) || - (*MadtInterruptControllerLength < 4)) { + // Check if the values used to control the parsing logic have been + // successfully read. + if ((MadtInterruptControllerType =3D=3D NULL) || + (MadtInterruptControllerLength =3D=3D NULL)) { IncrementErrorCount (); Print ( - L"ERROR: Invalid Interrupt Controller Length," - L" Type =3D %d, Length =3D %d\n", - *MadtInterruptControllerType, - *MadtInterruptControllerLength - ); - break; + L"ERROR: Insufficient remaining table buffer length to read the " \ + L"Interrupt Controller Structure header. Length =3D %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 =3D %d. " \ + L"MadtInterruptControllerType =3D %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 =3D %d. " \ + L"RemainingTableBufferLength =3D %d. MADT parsing aborted.\n", + *MadtInterruptControllerLength, + AcpiTableLength - Offset + ); + return; } =20 switch (*MadtInterruptControllerType) { @@ -316,11 +314,11 @@ ParseAcpiMadt ( } =20 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 =3D %d\n", + L"ERROR: Only one GICD must be present. GICDCount =3D %d\n", GICDCount ); } @@ -372,13 +370,16 @@ ParseAcpiMadt ( } =20 default: { - IncrementErrorCount (); - Print ( - L"ERROR: Unknown Interrupt Controller Structure," - L" Type =3D %d, Length =3D %d\n", - *MadtInterruptControllerType, - *MadtInterruptControllerLength - ); + if (GetConsistencyChecking ()) { + IncrementErrorCount (); + Print ( + L"ERROR: Unknown Interrupt Controller Structure," + L" Type =3D %d, Length =3D %d\n", + *MadtInterruptControllerType, + *MadtInterruptControllerLength + ); + } + break; } } // switch =20 -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43647): https://edk2.groups.io/g/devel/message/43647 Mute This Topic: https://groups.io/mt/32439510/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-