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.136, mailfrom: shenglei.zhang@intel.com) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by groups.io with SMTP; Wed, 04 Sep 2019 19:11:08 -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 fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Sep 2019 19:11:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,469,1559545200"; d="scan'208";a="212615702" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga002.fm.intel.com with ESMTP; 04 Sep 2019 19:11:07 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Sep 2019 19:11:07 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 4 Sep 2019 19:11:06 -0700 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 4 Sep 2019 19:11:06 -0700 Received: from shsmsx106.ccr.corp.intel.com ([169.254.10.86]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.195]) with mapi id 14.03.0439.000; Thu, 5 Sep 2019 10:11:05 +0800 From: "Zhang, Shenglei" To: "Chiu, Chasel" , "devel@edk2.groups.io" 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: AQHVYYrWoiM71jMSU0ydmmayYdsKMacbb9XggADfRNA= Date: Thu, 5 Sep 2019 02:11:04 +0000 Message-ID: References: <20190902123452.27716-1-shenglei.zhang@intel.com> <3C3EFB470A303B4AB093197B6777CCEC5048145B@PGSMSX111.gar.corp.intel.com> In-Reply-To: <3C3EFB470A303B4AB093197B6777CCEC5048145B@PGSMSX111.gar.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: shenglei.zhang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi chasel, > -----Original Message----- > From: Chiu, Chasel > Sent: Wednesday, September 4, 2019 8:13 PM > To: devel@edk2.groups.io; Zhang, Shenglei > Cc: Kubacki, Michael A ; Desimone, Nathanie= l > L ; Gao, Liming > Subject: RE: [edk2-devel] [PATCH] > MinPlatformPkg/SecFspWrapperPlatformSecLib: Change > TopOfTemporaryRam type >=20 >=20 > Hi Shenglei, >=20 > Would you please elaborate a little on how casting to UINTN can resolve = the > overflow scenario and why 64bits OS will affect this code? Actually casting to UINTN can't resolve the overflow. What I mean is the result of (TopOfTemporaryRam - sizeof (UINT32)) may ove= rflow. While it's meaningless to cast an already overflowed result to another typ= e. So I update the code to cast the variable to UINT before it is arithmetically operated= . Under 64bits OS, the size of UINTN is 64-bit and the original "(TopOfTempo= raryRam - sizeof (UINT32)) " is 32-bit. So the operation for casting will be performed. That's why 64bi= ts OS will affect this code. Thanks, Shenglei >=20 > Thanks! > Chasel >=20 > > -----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 > > > > 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. > > > > 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(-) > > > > diff --git > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf > or > > mSecLib/SecGetPerformance.c > > > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf > or > > mSecLib/SecGetPerformance.c > > index c4eeb2b1..0cc42f96 100644 > > --- > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf > or > > 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 - size= of > > (UINT32)); > > + Count =3D *(UINT32 *)((UINTN)TopOfTemporaryRam - sizeof > > (UINT32)); > > Size =3D Count * sizeof (UINT32); > > > > Ticker =3D *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32)= - > Size > > - sizeof (UINT32) * 2); diff --git > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf > or > > mSecLib/SecPlatformInformation.c > > > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf > or > > mSecLib/SecPlatformInformation.c > > index 5b94ed2b..1bcee5f4 100644 > > --- > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf > or > > 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 - size= of > > (UINT32))); > > + Count =3D *((UINT32 *)((UINTN)TopOfTemporaryRam - sizeo= f > > (UINT32))); > > Size =3D Count * sizeof (IA32_HANDOFF_STATUS); > > > > if ((*StructureSize) < (UINT64) Size) { > > -- > > 2.18.0.windows.1 > > > > > >=20