From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web11.650.1580744175233397761 for ; Mon, 03 Feb 2020 07:36:15 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: zhichao.gao@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Feb 2020 07:36:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,398,1574150400"; d="scan'208";a="231057820" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga003.jf.intel.com with ESMTP; 03 Feb 2020 07:36:14 -0800 Received: from shsmsx606.ccr.corp.intel.com (10.109.6.216) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 3 Feb 2020 07:36:13 -0800 Received: from shsmsx603.ccr.corp.intel.com (10.109.6.143) by SHSMSX606.ccr.corp.intel.com (10.109.6.216) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 3 Feb 2020 23:36:12 +0800 Received: from shsmsx603.ccr.corp.intel.com ([10.109.6.143]) by SHSMSX603.ccr.corp.intel.com ([10.109.6.143]) with mapi id 15.01.1713.004; Mon, 3 Feb 2020 23:36:12 +0800 From: "Gao, Zhichao" To: Krzysztof Koch , "devel@edk2.groups.io" CC: "Ni, Ray" , "Sami.Mujawar@arm.com" , "Matteo.Carlini@arm.com" , "nd@arm.com" Subject: Re: [PATCH v3 00/11] Test against invalid pointers in acpiview Thread-Topic: [PATCH v3 00/11] Test against invalid pointers in acpiview Thread-Index: AQHVz4LIo5aAAwNpU0SKaFN5YYs/XqgEUOFQ Date: Mon, 3 Feb 2020 15:36:11 +0000 Message-ID: <31f632dc6dc54b06980af57ae0ab4cf7@intel.com> References: <20200120111351.29184-1-krzysztof.koch@arm.com> In-Reply-To: <20200120111351.29184-1-krzysztof.koch@arm.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDgzODQ2ZDMtOWNmOS00ZGM2LTg5MTgtNTU4OGE5MDkyMWYyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQVArOWtNeEJSU1BpMklcLzNYQ1drd3JkV2lZS0QrQ2o1K3ZSZjVMNmpZRHhFSkk2b2grcG1OVTJwdkZIZEVNelMifQ== dlp-reaction: no-action dlp-version: 11.2.0.6 x-originating-ip: [10.239.127.36] MIME-Version: 1.0 Return-Path: zhichao.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sorry for the misunderstanding before. The patch set is good to me. Series: Reviewed-by: Zhichao Gao Thanks, Zhichao > -----Original Message----- > From: Krzysztof Koch [mailto:krzysztof.koch@arm.com] > Sent: Monday, January 20, 2020 7:14 PM > To: devel@edk2.groups.io > Cc: Ni, Ray ; Gao, Zhichao ; > Sami.Mujawar@arm.com; Matteo.Carlini@arm.com; nd@arm.com > Subject: [PATCH v3 00/11] Test against invalid pointers in acpiview >=20 > Prevent the use of invalid pointers when parsing ACPI tables in the UEFI = shell > acpiview tool. >=20 > The parsing of ACPI tables is often controlled with the values read earli= er from > the same table. For example, the 'Offset' or 'Count' fields found in a st= ructure > are later used to parse the substructures. If such fields lie outside the= structure's > buffer length provided, then there is a possibility for a wild or danglin= g pointer. >=20 > Currently, if the ParseAcpi() function terminates early because the end o= f the > input table data buffer has been reached, then the pointers which were > supposed to be updated by this function are left untouched. > This is a security issue as the values pointed to by these pointers are l= ater used > for flow control. >=20 > This patch series aims to solve this security issue by explicitly initial= izing any > pointers lying outside the input ACPI data buffer to NULL and testing for= NULL > whenever these pointers are dereferenced. >=20 > Changes can be seet at: > https://github.com/KrzysztofKoch1/edk2/tree/612_add_pointer_validation_v3 >=20 > Notes: > v3: > - Rebase on latest master [Krzysztof] >=20 > v2: > - Do not require FadtMinorRevision and X_DsdtAddress pointers to be > valid in FADT table parser [Zhichao] >=20 > v1: > - Validate static pointers in acpiview parsers before use [Krzysztof] >=20 > Krzysztof Koch (11): > ShellPkg: acpiview: Set ItemPtr to NULL for unprocessed table fields > ShellPkg: acpiview: RSDP: Validate global pointer before use > ShellPkg: acpiview: FADT: Validate global pointer before use > ShellPkg: acpiview: SLIT: Validate global pointer before use > ShellPkg: acpiview: SLIT: Validate System Locality count > ShellPkg: acpiview: SRAT: Validate global pointers before use > ShellPkg: acpiview: MADT: Validate global pointers before use > ShellPkg: acpiview: PPTT: Validate global pointers before use > ShellPkg: acpiview: IORT: Validate global pointers before use > ShellPkg: acpiview: GTDT: Validate global pointers before use > ShellPkg: acpiview: DBG2: Validate global pointers before use >=20 > ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c |= 9 ++- > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | > 43 ++++++++++++++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | = 21 > +++---- > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | = 37 > ++++++++++++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | = 52 > +++++++++++++++++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | > 13 +++++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | = 25 > ++++++++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c | > 12 ++++ > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c | = 61 > ++++++++++++++++++-- > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | = 13 > +++++ > 10 files changed, 269 insertions(+), 17 deletions(-) >=20 > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' >=20