From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web10.6584.1592298291128789206 for ; Tue, 16 Jun 2020 02:04:51 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: zhiguang.liu@intel.com) IronPort-SDR: 0cqOJ4a2Twc0/Cd26BW9FZ5qrUvaG4ffUniWkQIuU4UFJ0QzJKP+fjpaC6dy9f1v2YcwvsyMfs BGZdWfAp9+rg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jun 2020 02:04:41 -0700 IronPort-SDR: gwBeGFCOmxFtH/a+jgPXGxXTnWX96MMXn/AioYhfAwZAsDzuzNwFSuP27aM+N/CXkl+psKVzz6 oRH8qkS9VYKQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,518,1583222400"; d="scan'208";a="290992283" Received: from fieedk002.ccr.corp.intel.com ([10.239.158.178]) by orsmga002.jf.intel.com with ESMTP; 16 Jun 2020 02:04:36 -0700 From: "Zhiguang Liu" To: devel@edk2.groups.io Cc: Star Zeng , Liming Gao , Jian J Wang , Hao A Wu Subject: [PATCH 1/5] MdeModulePkg: avoid SMM pointers being leaked by not using CopyMem() Date: Tue, 16 Jun 2020 17:04:30 +0800 Message-Id: <20200616090434.1201-1-zhiguang.liu@intel.com> X-Mailer: git-send-email 2.25.1.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2002 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. Doing so will prevent SMM pointers address from being leaked into non-SMM environment. Please note that newly introduced internal function CopyVarInfoEntry() will 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. Cc: Star Zeng Cc: Liming Gao Cc: Jian J Wang Signed-off-by: Hao A Wu Signed-off-by: Zhiguang Liu --- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 33 ++++++++++++= +++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/Mde= ModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c index caca5c3241..74e756bc00 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c @@ -315,6 +315,35 @@ GetFvbCountAndBuffer ( }=0D =0D =0D +/**=0D + Copy only the meaningful fields of the variable statistics information f= rom=0D + source buffer to the destination buffer. Other fields are filled with ze= ro.=0D +=0D + @param[out] DstInfoEntry A pointer to the buffer of destination vari= able=0D + information entry.=0D + @param[in] SrcInfoEntry A pointer to the buffer of source variable= =0D + information entry.=0D +=0D +**/=0D +static=0D +VOID=0D +CopyVarInfoEntry (=0D + OUT VARIABLE_INFO_ENTRY *DstInfoEntry,=0D + IN VARIABLE_INFO_ENTRY *SrcInfoEntry=0D + )=0D +{=0D + DstInfoEntry->Next =3D NULL;=0D + DstInfoEntry->Name =3D NULL;=0D +=0D + CopyGuid (&DstInfoEntry->VendorGuid, &SrcInfoEntry->VendorGuid);=0D + DstInfoEntry->Attributes =3D SrcInfoEntry->Attributes;=0D + DstInfoEntry->ReadCount =3D SrcInfoEntry->ReadCount;=0D + DstInfoEntry->WriteCount =3D SrcInfoEntry->WriteCount;=0D + DstInfoEntry->DeleteCount =3D SrcInfoEntry->DeleteCount;=0D + DstInfoEntry->CacheCount =3D SrcInfoEntry->CacheCount;=0D + DstInfoEntry->Volatile =3D SrcInfoEntry->Volatile;=0D +}=0D +=0D /**=0D Get the variable statistics information from the information buffer poin= ted by gVariableInfo.=0D =0D @@ -377,7 +406,7 @@ SmmVariableGetStatistics ( *InfoSize =3D StatisticsInfoSize;=0D return EFI_BUFFER_TOO_SMALL;=0D }=0D - CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));=0D + CopyVarInfoEntry (InfoEntry, VariableInfo);=0D CopyMem (InfoName, VariableInfo->Name, NameSize);=0D *InfoSize =3D StatisticsInfoSize;=0D return EFI_SUCCESS;=0D @@ -417,7 +446,7 @@ SmmVariableGetStatistics ( return EFI_BUFFER_TOO_SMALL;=0D }=0D =0D - CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));=0D + CopyVarInfoEntry (InfoEntry, VariableInfo);=0D CopyMem (InfoName, VariableInfo->Name, NameSize);=0D *InfoSize =3D StatisticsInfoSize;=0D =0D --=20 2.25.1.windows.1