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.120, mailfrom: star.zeng@intel.com) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by groups.io with SMTP; Tue, 06 Aug 2019 07:55:55 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Aug 2019 07:55:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,353,1559545200"; d="scan'208";a="374078028" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga006.fm.intel.com with ESMTP; 06 Aug 2019 07:55:55 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 6 Aug 2019 07:55:55 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 6 Aug 2019 07:55:54 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.19]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.163]) with mapi id 14.03.0439.000; Tue, 6 Aug 2019 22:55:53 +0800 From: "Zeng, Star" To: "devel@edk2.groups.io" , "Wu, Hao A" CC: "Gao, Liming" , "Wang, Jian J" , "Zeng, Star" Subject: Re: [edk2-devel] [PATCH v1] MdeModulePkg/Variable/RuntimeDxe: Not copy SMM pointers in comm buffer Thread-Topic: [edk2-devel] [PATCH v1] MdeModulePkg/Variable/RuntimeDxe: Not copy SMM pointers in comm buffer Thread-Index: AQHVTCcupuDPuC8yS0i5yNTofIdONKbuNWeQ Date: Tue, 6 Aug 2019 14:55:52 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483104038E284@shsmsx102.ccr.corp.intel.com> References: <20190806071832.30108-1-hao.a.wu@intel.com> In-Reply-To: <20190806071832.30108-1-hao.a.wu@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTcyODJhMTktYWViMy00MWQyLWJhNjQtZmY4YzBiNDhkNmFiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUG1WVHV5OWR5dHFYVFoxTUhuTmZab09qMnY1NDlvTG5VT2s5MklpNllOMERob0lLSTRyNk42a1pzN3lvTEJ2NiJ9 dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: star.zeng@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The DstInfoEntry is better to have identifer [out] instead of [in]? Thanks, Star > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Wu= , > Hao A > Sent: Tuesday, August 6, 2019 3:19 PM > To: devel@edk2.groups.io > Cc: Wu, Hao A ; Gao, Liming ; > Wang, Jian J > Subject: [edk2-devel] [PATCH v1] MdeModulePkg/Variable/RuntimeDxe: Not > copy SMM pointers in comm buffer >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2002 >=20 > This commit will update the logic in function SmmVariableGetStatistics()= so > that the pointer fields ('Next' and 'Name') in structure > VARIABLE_INFO_ENTRY will not be copied into the SMM communication > buffer. >=20 > Doing so will prevent SMM pointers address from being leaked into non- > SMM environment. >=20 > Please note that newly introduced internal function CopyVarInfoEntry() w= ill > not use CopyMem() to copy the whole VARIABLE_INFO_ENTRY structure and > then zero out the 'Next' and 'Name' fields. This is for preventing race > conditions where the pointers value might still be read. >=20 > Cc: Liming Gao > Cc: Jian J Wang > Signed-off-by: Hao A Wu > --- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 33 > ++++++++++++++++++-- > 1 file changed, 31 insertions(+), 2 deletions(-) >=20 > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > index ec463d063e..81a9c9e849 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > @@ -312,6 +312,35 @@ GetFvbCountAndBuffer ( >=20 >=20 > /** > + Copy only the meaningful fields of the variable statistics > + information from source buffer to the destination buffer. Other field= s are > filled with zero. > + > + @param[in] DstInfoEntry A pointer to the buffer of destination va= riable > + information entry. > + @param[in] SrcInfoEntry A pointer to the buffer of source variabl= e > + information entry. > + > +**/ > +static > +VOID > +CopyVarInfoEntry ( > + IN VARIABLE_INFO_ENTRY *DstInfoEntry, > + IN VARIABLE_INFO_ENTRY *SrcInfoEntry > + ) > +{ > + DstInfoEntry->Next =3D NULL; > + DstInfoEntry->Name =3D NULL; > + > + CopyGuid (&DstInfoEntry->VendorGuid, &SrcInfoEntry->VendorGuid); > + DstInfoEntry->Attributes =3D SrcInfoEntry->Attributes; > + DstInfoEntry->ReadCount =3D SrcInfoEntry->ReadCount; > + DstInfoEntry->WriteCount =3D SrcInfoEntry->WriteCount; > + DstInfoEntry->DeleteCount =3D SrcInfoEntry->DeleteCount; > + DstInfoEntry->CacheCount =3D SrcInfoEntry->CacheCount; > + DstInfoEntry->Volatile =3D SrcInfoEntry->Volatile; > +} > + > +/** > Get the variable statistics information from the information buffer p= ointed > by gVariableInfo. >=20 > Caution: This function may be invoked at SMM runtime. > @@ -373,7 +402,7 @@ SmmVariableGetStatistics ( > *InfoSize =3D StatisticsInfoSize; > return EFI_BUFFER_TOO_SMALL; > } > - CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY)); > + CopyVarInfoEntry (InfoEntry, VariableInfo); > CopyMem (InfoName, VariableInfo->Name, NameSize); > *InfoSize =3D StatisticsInfoSize; > return EFI_SUCCESS; > @@ -413,7 +442,7 @@ SmmVariableGetStatistics ( > return EFI_BUFFER_TOO_SMALL; > } >=20 > - CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY)); > + CopyVarInfoEntry (InfoEntry, VariableInfo); > CopyMem (InfoName, VariableInfo->Name, NameSize); > *InfoSize =3D StatisticsInfoSize; >=20 > -- > 2.12.0.windows.1 >=20 >=20 >=20