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=fxFkhkY2; spf=pass (domain: arm.com, ip: 40.107.4.58, mailfrom: krzysztof.koch@arm.com) Received: from EUR03-DB5-obe.outbound.protection.outlook.com (EUR03-DB5-obe.outbound.protection.outlook.com [40.107.4.58]) by groups.io with SMTP; Wed, 17 Jul 2019 06:38:31 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=D2Eo4AoaPoCOnPWXDghX2/s5w/aGBZ1FFrczKErjjmhqZ4w2+ejRJ+EYdB56PYs0j59UI7UFC2dMjM87ObiN6L/nmHvmoK8t9WDAE7D32kD0u0bEiMRDtZ5A9WaUgbrX7gqNOclR0KkrBrJs55xOHuBcvAsp5vrzR5W1HA166Sw3P2TQ59i5jLKxw33SikE+1wQOgqOd+dsGEMyOSCy+5+q4MMTijKCF2qv+cP1S8NcGnyjMJ/Ca9a/bM4xcpNFqv3kwbby4rnTjotSIs/TWPtwG/xpPexgqTaX5+c7SFacg3Fr7xjhy1NF/pTXpXktk8GFEnDWt002DwYbjCeyRVA== 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=1iha6+zwPOC8U/GGc/meCiVpo3o8oRbJB+vRjRH0ZR0=; b=Ah4+/ofATtaKDbJhiHAY+smYNq4uUaecxPTG4L1EON/FoTeANIRC6Y8I4nH/eAUOEUPgFJMnQN4qTQNgV9oH+kxAzBuT1TClO45Nj03DU8DFpipy8UvkAOFQG+tk/Yrjyoia+5cr9Dd60L+IOv3Dpan12ncyFmTCq5mokCIn4HBGrHaGxVTWlfZW9EJwi8HxpqLRQsW5g0ivzSseDepRyihqYwXxUEJztNXCGRbeaxB4+g3zjlDG/ftqpH7Be4A3OLRVtg1zLyECHm10ffom7Mb4gHw9TIaMFIesWs68FvjaDbEF4luDuQ8GSLHq+XbfOrFvGNRn3amhUYN9FeJ26Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=arm.com;dmarc=pass action=none header.from=arm.com;dkim=pass header.d=arm.com;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=1iha6+zwPOC8U/GGc/meCiVpo3o8oRbJB+vRjRH0ZR0=; b=fxFkhkY2RNhKm5nNakG5qHhtVmsr8mWyImQvNiQ745YmqoZNBQLSk0TlP5guNxSm6qZ3jhOgpiOkJemtqF+DRNOb6+4OI9SUa/2+xloJBFDDE3RNjcfkVuTX8YY3/MsF84CuYSY804m59eo7shyXYxmcW8ZJ8TSFGgJxGuMQ8Ng= Received: from VE1PR08MB4783.eurprd08.prod.outlook.com (10.255.114.16) by VE1PR08MB5229.eurprd08.prod.outlook.com (10.255.27.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2073.14; Wed, 17 Jul 2019 13:38:28 +0000 Received: from VE1PR08MB4783.eurprd08.prod.outlook.com ([fe80::9554:1492:6d83:be6b]) by VE1PR08MB4783.eurprd08.prod.outlook.com ([fe80::9554:1492:6d83:be6b%4]) with mapi id 15.20.2073.012; Wed, 17 Jul 2019 13:38:28 +0000 From: "Krzysztof Koch" To: "devel@edk2.groups.io" , "jaben.carsey@intel.com" Subject: Re: [edk2-devel] [PATCH v1 01/11] ShellPkg: acpiview: FADT: Validate global pointers before use Thread-Topic: [edk2-devel] [PATCH v1 01/11] ShellPkg: acpiview: FADT: Validate global pointers before use Thread-Index: AQHVOL3TE6b7YtUdxkeJXpJv26gsnqbOxj6w Date: Wed, 17 Jul 2019 13:38:28 +0000 Message-ID: References: <20190712065243.3812-1-krzysztof.koch@arm.com> <20190712065243.3812-2-krzysztof.koch@arm.com> In-Reply-To: Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: 49c6d43e-c854-4bec-9d8c-0ed4d49ae3f3.1 x-checkrecipientchecked: true authentication-results: spf=none (sender IP is ) smtp.mailfrom=Krzysztof.Koch@arm.com; x-originating-ip: [217.140.106.53] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: f6327fd1-8cbc-4f1d-6436-08d70abc0d02 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(4618075)(2017052603328)(7193020);SRVR:VE1PR08MB5229; x-ms-traffictypediagnostic: VE1PR08MB5229: x-ms-exchange-purlcount: 4 x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 01018CB5B3 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(4636009)(39860400002)(136003)(346002)(376002)(366004)(396003)(40434004)(13464003)(189003)(199004)(64756008)(478600001)(6306002)(15650500001)(66946007)(76116006)(66476007)(66446008)(5660300002)(6436002)(55016002)(66556008)(53546011)(68736007)(14444005)(5024004)(6246003)(99286004)(9686003)(6506007)(2906002)(316002)(8936002)(81156014)(81166006)(53936002)(229853002)(52536014)(8676002)(3846002)(486006)(446003)(11346002)(25786009)(2501003)(7736002)(305945005)(102836004)(476003)(71190400001)(66066001)(74316002)(86362001)(33656002)(966005)(76176011)(256004)(186003)(71200400001)(14454004)(110136005)(26005)(6116002)(7696005);DIR:OUT;SFP:1101;SCL:1;SRVR:VE1PR08MB5229;H:VE1PR08MB4783.eurprd08.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: 4CSFbDK/ckH3Dsx+rVWLKroj4XaLKeeRymWU3T5ZE9nhnPEnfE5xdYHoOKOcqiT3Um1wzwdWjmLId7UxigIHn57FhrW69coo17BOXhf6KMFGzhjiU9qxieTq3hurtF+EYYsRyaIdbwQwj4emDu+Aca5S32Ob5MxAWrfpjvuu5RGDgGSSzSybrWMv16Yjx4IjcsVbZgEeaHmg5THZJFpBDrrzbx7TRtU+501JgbZWTnFg91a+d3QkMh8ahhwD08bFftvj3aHHvFm4bXAgnLuNDPMAepvIWd46ObjGx9e7117ZfP2lvmXct4vKKxZE/agjsc3Q0B1gpoVMCNxt5xQ/M0lfH0aaktWjBg7XV4KDXzYMX5SGKUSIgZo1Dy0nvvZWofGmEkYaV0fEPybDRE3RoyDw8EIpa3hjeh+nyt2THTk= MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: f6327fd1-8cbc-4f1d-6436-08d70abc0d02 X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Jul 2019 13:38:28.3145 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Krzysztof.Koch@arm.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR08MB5229 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Jaben, I will split the changes into separate patch sets with each patch set havi= ng the same logical change made to every applicable acpiview table parser. = The per-parser modifications will be separate commits as well. Kind regards, Krzysztof -----Original Message----- From: devel@edk2.groups.io On Behalf Of Carsey, Jab= en via Groups.Io Sent: Friday, July 12, 2019 15:27 To: Krzysztof Koch ; devel@edk2.groups.io Cc: Ni, Ray ; Gao, Zhichao ; Sami= Mujawar ; Matteo Carlini ; n= d Subject: Re: [edk2-devel] [PATCH v1 01/11] ShellPkg: acpiview: FADT: Valid= ate global pointers before use I think it would be easier to see/review these changes logically if the fu= nctional changes (1 and 3) were separate from the refactoring change (2). Reviewed-by: Jaben Carsey > -----Original Message----- > From: Krzysztof Koch [mailto:krzysztof.koch@arm.com] > Sent: Thursday, July 11, 2019 11:53 PM > To: devel@edk2.groups.io > Cc: Carsey, Jaben ; Ni, Ray > ; Gao, Zhichao ; > Sami.Mujawar@arm.com; Matteo.Carlini@arm.com; nd@arm.com > Subject: [PATCH v1 01/11] ShellPkg: acpiview: FADT: Validate global > pointers before use > > 1. Check if the global pointer have been successfully updated before > they are later used to control the parsing logic in the FADT acpiview > parser. > > 2. Remove redundant forward function declarations by repositioning > blocks of code. > > 3. 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/49cc41430775fb93205e302 > 590a7d31f080c3952 > > Notes: > v1: > - improve the logic in the parser [Krzysztof] > > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c > | 131 ++++++++------------ > 1 file changed, 51 insertions(+), 80 deletions(-) > > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > index > cee7ee0770433da96d6042d2f5d687903f4b5495..600d3b16d7b22b61c1a1fd21 > ecb93f16c7f8fa1a 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > +++ > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > @@ -1,7 +1,7 @@ > /** @file > FADT 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): > @@ -12,6 +12,7 @@ > #include > #include "AcpiParser.h" > #include "AcpiTableParser.h" > +#include "AcpiView.h" > > // Local variables > STATIC CONST UINT32* DsdtAddress; > @@ -46,7 +47,17 @@ EFIAPI > ValidateFirmwareCtrl ( > IN UINT8* Ptr, > IN VOID* Context > - ); > +) > +{ > +#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) > + if (*(UINT32*)Ptr !=3D 0) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: Firmware Control must be zero for ARM platforms." > + ); > + } > +#endif > +} > > /** > This function validates the X_Firmware Control Field. > @@ -61,7 +72,17 @@ EFIAPI > ValidateXFirmwareCtrl ( > IN UINT8* Ptr, > IN VOID* Context > - ); > +) > +{ > +#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) > + if (*(UINT64*)Ptr !=3D 0) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: X Firmware Control must be zero for ARM platforms." > + ); > + } > +#endif > +} > > /** > This function validates the flags. > @@ -76,7 +97,17 @@ EFIAPI > ValidateFlags ( > IN UINT8* Ptr, > IN VOID* Context > - ); > +) > +{ > +#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) > + if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) =3D=3D 0) { > + IncrementErrorCount (); > + Print ( > + L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms." > + ); > + } > +#endif > +} > > /** > An ACPI_PARSER array describing the ACPI FADT Table. > @@ -142,81 +173,6 @@ STATIC CONST ACPI_PARSER FadtParser[] =3D { > {L"Hypervisor VendorIdentity", 8, 268, L"%lx", NULL, NULL, NULL, > NULL} }; > > -/** > - This function validates the Firmware Control Field. > - > - @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 > -ValidateFirmwareCtrl ( > - IN UINT8* Ptr, > - IN VOID* Context > -) > -{ > -#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) > - if (*(UINT32*)Ptr !=3D 0) { > - IncrementErrorCount (); > - Print ( > - L"\nERROR: Firmware Control must be zero for ARM platforms." > - ); > - } > -#endif > -} > - > -/** > - This function validates the X_Firmware Control Field. > - > - @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 > -ValidateXFirmwareCtrl ( > - IN UINT8* Ptr, > - IN VOID* Context > -) > -{ > -#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) > - if (*(UINT64*)Ptr !=3D 0) { > - IncrementErrorCount (); > - Print ( > - L"\nERROR: X Firmware Control must be zero for ARM platforms." > - ); > - } > -#endif > -} > - > -/** > - This function validates the flags. > - > - @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 > -ValidateFlags ( > - IN UINT8* Ptr, > - IN VOID* Context > -) > -{ > -#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) > - if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) =3D=3D 0) { > - IncrementErrorCount (); > - Print ( > - L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms." > - ); > - } > -#endif > -} > - > /** > This function parses the ACPI FADT table. > This function parses the FADT table and optionally traces the ACPI > table fields. > @@ -248,12 +204,27 @@ ParseAcpiFadt ( > PARSER_PARAMS (FadtParser) > ); > > + // Check if the values used to control the parsing logic have been > + // successfully read. > + if ((DsdtAddress =3D=3D NULL) || > + (FadtMinorRevision =3D=3D NULL) || > + (X_DsdtAddress =3D=3D NULL)) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Insufficient table length. AcpiTableLength =3D %d. " \ > + L"FADT parsing aborted.\n", > + AcpiTableLength > + ); > + return; > + } > + > if (Trace) { > Print (L"\nSummary:\n"); > PrintFieldName (2, L"FADT Version"); > Print (L"%d.%d\n", *AcpiHdrInfo.Revision, *FadtMinorRevision); > > - if (*GetAcpiXsdtHeaderInfo ()->OemTableId !=3D > *AcpiHdrInfo.OemTableId) { > + if (GetConsistencyChecking () && > + (*GetAcpiXsdtHeaderInfo ()->OemTableId !=3D > *AcpiHdrInfo.OemTableId)) { > IncrementErrorCount (); > Print (L"ERROR: OEM Table Id does not match with RSDT/XSDT.\n"); > } > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > IMPORTANT NOTICE: The contents of this email and any attachments are confi= dential and may also be privileged. If you are not the intended recipient, = please notify the sender immediately and do not disclose the contents to an= y other person, use it for any purpose, or store or copy the information in= any medium. Thank you.