From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A938081C81 for ; Mon, 21 Nov 2016 22:25:18 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP; 21 Nov 2016 22:25:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,531,1473145200"; d="scan'208";a="194327904" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 21 Nov 2016 22:25:18 -0800 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Nov 2016 22:25:18 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Nov 2016 22:25:17 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.138]) with mapi id 14.03.0248.002; Tue, 22 Nov 2016 14:25:15 +0800 From: "Yao, Jiewen" To: "Fan, Jeff" , "edk2-devel@lists.01.org" CC: "Tian, Feng" , "Kinney, Michael D" Thread-Topic: [PATCH] MdeModulePkg/PiSmmCore: Cache CommunicationBuffer info before using it Thread-Index: AQHSQWm1fkLEqleKnUiv8DL3KNPanaDkj4Bg Date: Tue, 22 Nov 2016 06:25:14 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50386D89AD@shsmsx102.ccr.corp.intel.com> References: <20161118070152.16716-1-jeff.fan@intel.com> In-Reply-To: <20161118070152.16716-1-jeff.fan@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/PiSmmCore: Cache CommunicationBuffer info before using it X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Nov 2016 06:25:18 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jiewen.yao@intel.com > -----Original Message----- > From: Fan, Jeff > Sent: Friday, November 18, 2016 3:02 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Tian, Feng ; > Kinney, Michael D > Subject: [PATCH] MdeModulePkg/PiSmmCore: Cache CommunicationBuffer > info before using it >=20 > gSmmCorePrivate->CommunicationBuffer and > gSmmCorePrivate->BufferSize locate at > runtime memory region. That means they could be modified by non-SMM > code during > runtime. >=20 > We should cache them into SMM local variables before we verify them. Afte= r > verification, we should use the cached ones directly instead of the ones = in > gSmmCorePrivate. >=20 > Cc: Jiewen Yao > Cc: Feng Tian > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan > --- > MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) >=20 > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c > b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c > index b877a33..de8db65 100644 > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c > @@ -432,6 +432,8 @@ SmmEntryPoint ( > EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader; > BOOLEAN InLegacyBoot; > BOOLEAN IsOverlapped; > + VOID *CommunicationBuffer; > + UINTN BufferSize; >=20 > PERF_START (NULL, "SMM", NULL, 0) ; >=20 > @@ -463,17 +465,19 @@ SmmEntryPoint ( > // Check to see if this is a Synchronous SMI sent through the SMM > Communication > // Protocol or an Asynchronous SMI > // > - if (gSmmCorePrivate->CommunicationBuffer !=3D NULL) { > + CommunicationBuffer =3D gSmmCorePrivate->CommunicationBuffer; > + BufferSize =3D gSmmCorePrivate->BufferSize; > + if (CommunicationBuffer !=3D NULL) { > // > // Synchronous SMI for SMM Core or request from Communicate > protocol > // > IsOverlapped =3D InternalIsBufferOverlapped ( > - (UINT8 *) > gSmmCorePrivate->CommunicationBuffer, > - gSmmCorePrivate->BufferSize, > + (UINT8 *) CommunicationBuffer, > + BufferSize, > (UINT8 *) gSmmCorePrivate, > sizeof (*gSmmCorePrivate) > ); > - if (!SmmIsBufferOutsideSmmValid > ((UINTN)gSmmCorePrivate->CommunicationBuffer, > gSmmCorePrivate->BufferSize) || IsOverlapped) { > + if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, > BufferSize) || IsOverlapped) { > // > // If CommunicationBuffer is not in valid address scope, > // or there is overlap between gSmmCorePrivate and > CommunicationBuffer, > @@ -482,19 +486,19 @@ SmmEntryPoint ( > gSmmCorePrivate->CommunicationBuffer =3D NULL; > gSmmCorePrivate->ReturnStatus =3D EFI_INVALID_PARAMETER; > } else { > - CommunicateHeader =3D (EFI_SMM_COMMUNICATE_HEADER > *)gSmmCorePrivate->CommunicationBuffer; > - gSmmCorePrivate->BufferSize -=3D OFFSET_OF > (EFI_SMM_COMMUNICATE_HEADER, Data); > + CommunicateHeader =3D (EFI_SMM_COMMUNICATE_HEADER > *)CommunicationBuffer; > + BufferSize -=3D OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, > Data); > Status =3D SmiManage ( > &CommunicateHeader->HeaderGuid, > NULL, > CommunicateHeader->Data, > - &gSmmCorePrivate->BufferSize > + &BufferSize > ); > // > // Update CommunicationBuffer, BufferSize and ReturnStatus > // Communicate service finished, reset the pointer to > CommBuffer to NULL > // > - gSmmCorePrivate->BufferSize +=3D OFFSET_OF > (EFI_SMM_COMMUNICATE_HEADER, Data); > + gSmmCorePrivate->BufferSize =3D BufferSize + OFFSET_OF > (EFI_SMM_COMMUNICATE_HEADER, Data); > gSmmCorePrivate->CommunicationBuffer =3D NULL; > gSmmCorePrivate->ReturnStatus =3D (Status =3D=3D EFI_SUCCESS) ? > EFI_SUCCESS : EFI_NOT_FOUND; > } > -- > 2.9.3.windows.2