From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.43, mailfrom: rangasai.v.chaganty@intel.com) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by groups.io with SMTP; Mon, 15 Jul 2019 16:17:37 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Jul 2019 16:17:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,494,1557212400"; d="scan'208";a="161239593" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by orsmga008.jf.intel.com with ESMTP; 15 Jul 2019 16:17:37 -0700 Received: from orsmsx126.amr.corp.intel.com (10.22.240.126) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 15 Jul 2019 16:17:36 -0700 Received: from orsmsx107.amr.corp.intel.com ([169.254.1.186]) by ORSMSX126.amr.corp.intel.com ([169.254.4.77]) with mapi id 14.03.0439.000; Mon, 15 Jul 2019 16:17:36 -0700 From: "Chaganty, Rangasai V" To: "Desimone, Nathaniel L" , "devel@edk2.groups.io" CC: "Chiu, Chasel" , "Kubacki, Michael A" Subject: Re: [edk2-platforms] [PATCH] KabylakeSiliconPkg: Possible out-of-bounds memory writes Thread-Topic: [edk2-platforms] [PATCH] KabylakeSiliconPkg: Possible out-of-bounds memory writes Thread-Index: AQHVO014KXedGR7jsEqtFYiyPBF2j6bMT5ow Date: Mon, 15 Jul 2019 23:17:35 +0000 Message-ID: References: <20190715203933.29256-1-nathaniel.l.desimone@intel.com> In-Reply-To: <20190715203933.29256-1-nathaniel.l.desimone@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjhmZjIwMWMtMGRmOS00NTVmLWJhYmUtNjIyZDBmYjVjZmY4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWTVJVVBEQ1wvbllvQUYrelA0bjBTMHZDVFFTTEtRQ3VnMEMzeDJNdTlHOVV3WFloeTlVZlhnRlhUSUFKZks1amkifQ== x-ctpclassification: CTP_NT x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Return-Path: rangasai.v.chaganty@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Looks good.=20 Can we assign a local variable for "EndPtr" and use that in the For loop, i= nstead of the expression and keep the implementation cleaner? EndPtr =3D CurrPtr + ((EFI_ACPI_COMMON_HEADER *) CurrPtr)->Length; for (DsdtPointer =3D CurrPtr; DsdtPointer < EndPtr; DsdtPointer++) { -----Original Message----- From: Desimone, Nathaniel L=20 Sent: Monday, July 15, 2019 1:40 PM To: devel@edk2.groups.io Cc: Chiu, Chasel ; Kubacki, Michael A ; Chaganty, Rangasai V Subject: [edk2-platforms] [PATCH] KabylakeSiliconPkg: Possible out-of-bound= s memory writes - Add check for the DSDT not existing. - Fixed logic errors in loop boundary check. Cc: Chasel Chiu Cc: Michael A Kubacki Cc: Sai Chaganty Co-authored-by: John Mathews Signed-off-by: Nate DeSimone --- .../Library/DxeAslUpdateLib/DxeAslUpdateLib.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Silicon/Intel/KabylakeSiliconPkg/Library/DxeAslUpdateLib/DxeAs= lUpdateLib.c b/Silicon/Intel/KabylakeSiliconPkg/Library/DxeAslUpdateLib/Dxe= AslUpdateLib.c index e6ab43db6d..a9611f750f 100644 --- a/Silicon/Intel/KabylakeSiliconPkg/Library/DxeAslUpdateLib/DxeAslUpdate= Lib.c +++ b/Silicon/Intel/KabylakeSiliconPkg/Library/DxeAslUpdateLib/DxeAslUpdate= Lib.c @@ -59,6 +59,7 @@ InitializeAslUpdateLib ( @param[in] Length - length of data to be overwritten =20 @retval EFI_SUCCESS - The function completed successfully. + @retval EFI_NOT_FOUND - Failed to locate AcpiTable. **/ EFI_STATUS UpdateNameAslCode ( @@ -99,11 +100,14 @@ UpdateNameAslCode ( /// Point to the beginning of the DSDT table /// CurrPtr =3D (UINT8 *) Table; + if (CurrPtr =3D=3D NULL) { + return EFI_NOT_FOUND; + } =20 /// /// Loop through the ASL looking for values that we must fix up. /// - for (DsdtPointer =3D CurrPtr; DsdtPointer <=3D (CurrPtr + ((EFI_ACPI_COM= MON_HEADER *) CurrPtr)->Length); DsdtPointer++) { + for (DsdtPointer =3D CurrPtr; DsdtPointer < (CurrPtr + ((EFI_ACPI_COMMON= _HEADER *) CurrPtr)->Length); DsdtPointer++) { /// /// Get a pointer to compare for signature /// --=20 2.17.1.windows.2