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.88, mailfrom: chasel.chiu@intel.com) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by groups.io with SMTP; Wed, 04 Sep 2019 05:13:29 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Sep 2019 05:13:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,465,1559545200"; d="scan'208";a="212372454" Received: from pgsmsx114.gar.corp.intel.com ([10.108.55.203]) by fmsmga002.fm.intel.com with ESMTP; 04 Sep 2019 05:13:28 -0700 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.22]) by pgsmsx114.gar.corp.intel.com ([169.254.4.237]) with mapi id 14.03.0439.000; Wed, 4 Sep 2019 20:13:27 +0800 From: "Chiu, Chasel" To: "devel@edk2.groups.io" , "Zhang, Shenglei" CC: "Kubacki, Michael A" , "Desimone, Nathaniel L" , "Gao, Liming" Subject: Re: [edk2-devel] [PATCH] MinPlatformPkg/SecFspWrapperPlatformSecLib: Change TopOfTemporaryRam type Thread-Topic: [edk2-devel] [PATCH] MinPlatformPkg/SecFspWrapperPlatformSecLib: Change TopOfTemporaryRam type Thread-Index: AQHVYYrWcH+ZB150L0CdngW7WLvkd6cbb9Xg Date: Wed, 4 Sep 2019 12:13:27 +0000 Message-ID: <3C3EFB470A303B4AB093197B6777CCEC5048145B@PGSMSX111.gar.corp.intel.com> References: <20190902123452.27716-1-shenglei.zhang@intel.com> In-Reply-To: <20190902123452.27716-1-shenglei.zhang@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: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiM2Y2YWFhNTItMjM2OC00NmM5LWE5ZjAtZjliMWJlMDJlMzJjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiXC92cFQxYSs4RG82NjQ5XC9ZWlBWcXV1VytUcWMyckQyUDJLcFcyRFZXTmR0OWpnXC9hbWRHckNLcVFPQWI4OVp0VyJ9 x-ctpclassification: CTP_NT x-originating-ip: [172.30.20.205] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Shenglei, Would you please elaborate a little on how casting to UINTN can resolve th= e overflow scenario and why 64bits OS will affect this code? Thanks! Chasel > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Zhang, > Shenglei > Sent: Monday, September 2, 2019 8:35 PM > To: devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Desimone, Nathaniel L > ; Gao, Liming > Subject: [edk2-devel] [PATCH] MinPlatformPkg/SecFspWrapperPlatformSecLib= : > Change TopOfTemporaryRam type >=20 > Cast TopOfTemporaryRam's from UINT32 to UINTN in the expression. > The original code (TopOfTemporaryRam - sizeof (UINT32)) may cause > overflow. As a result the operation under 64-bit OS environment, (UINT)(= ...), > may cast a overflowed 4-byte result to 8-byte one. >=20 > Cc: Michael Kubacki > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Liming Gao > Signed-off-by: Shenglei Zhang > --- > .../Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c | 2 +- > .../SecFspWrapperPlatformSecLib/SecPlatformInformation.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) >=20 > diff --git > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecGetPerformance.c > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecGetPerformance.c > index c4eeb2b1..0cc42f96 100644 > --- > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecGetPerformance.c > +++ > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat > +++ formSecLib/SecGetPerformance.c > @@ -79,7 +79,7 @@ SecGetPerformance ( > // > TopOfTemporaryRam =3D (UINT32)(UINTN)TopOfTemporaryRamPpi - > sizeof(UINT32); > TopOfTemporaryRam -=3D sizeof(UINT32) * 2; > - Count =3D *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof > (UINT32)); > + Count =3D *(UINT32 *)((UINTN)TopOfTemporaryRam - sizeof > (UINT32)); > Size =3D Count * sizeof (UINT32); >=20 > Ticker =3D *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) -= Size > - sizeof (UINT32) * 2); diff --git > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecPlatformInformation.c > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecPlatformInformation.c > index 5b94ed2b..1bcee5f4 100644 > --- > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecPlatformInformation.c > +++ > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat > +++ formSecLib/SecPlatformInformation.c > @@ -61,7 +61,7 @@ SecPlatformInformation ( > // > TopOfTemporaryRam =3D (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof > (UINT32); > TopOfTemporaryRam -=3D sizeof(UINT32) * 2; > - Count =3D *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof > (UINT32))); > + Count =3D *((UINT32 *)((UINTN)TopOfTemporaryRam - sizeof > (UINT32))); > Size =3D Count * sizeof (IA32_HANDOFF_STATUS); >=20 > if ((*StructureSize) < (UINT64) Size) { > -- > 2.18.0.windows.1 >=20 >=20 >=20